Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add tracked utilities for ember-inspector #1489

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

patricklx
Copy link
Contributor

@patricklx patricklx commented Nov 3, 2023

@patricklx patricklx force-pushed the tracked-utils branch 2 times, most recently from 25ec20f to 6e4232b Compare November 3, 2023 11:28
@patricklx
Copy link
Contributor Author

@NullVoxPopuli can we have this?

@NullVoxPopuli
Copy link
Contributor

Maybe, i'm behind an getting the VM integrated in to ember, so that's blocking me actually reviewing all these prs.

At first glance though, does this need to be in production? Can we instead wrap all the behavior in meta.env?

@patricklx
Copy link
Contributor Author

patricklx commented Nov 7, 2023

i think it would be nice to have it in prod as well.
or when window.EmberENV = { _DEBUG_RENDER_TREE: true }; is set? or some other key?
https://github.com/emberjs/ember-inspector/blob/main/skeletons/web-extension/scripts/boot.js#L1C21-L1C39

@patricklx patricklx force-pushed the tracked-utils branch 4 times, most recently from e403516 to 80c34c3 Compare November 28, 2023 13:58
@patricklx patricklx force-pushed the tracked-utils branch 2 times, most recently from 4cc0bff to e4a3192 Compare February 8, 2024 13:35
@patricklx patricklx changed the title add tag info for ember-inspector add tracked utilities for ember-inspector Feb 8, 2024
@patricklx patricklx force-pushed the tracked-utils branch 5 times, most recently from 79fa4e6 to 3318240 Compare February 12, 2024 11:43
@patricklx
Copy link
Contributor Author

@chancancode can you have a look at this? At least i would like the object/property info on the tag for inspector

@NullVoxPopuli
Copy link
Contributor

image

artifact-1.pdf

@NullVoxPopuli
Copy link
Contributor

Is there a way to make it faster?

@patricklx patricklx closed this Jun 11, 2024
@patricklx patricklx reopened this Jun 11, 2024
@patricklx
Copy link
Contributor Author

Is there a way to make it faster?

Maybe, nodejs creates a new hidden class whenever a new property is added to an object... Lets see if thats the cause

@NullVoxPopuli
Copy link
Contributor

image

artifact-1.pdf

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks for the tests!! 🎉

export { trackedData } from './lib/tracked-data';
export * from './lib/tracked-utils';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

since we export * here, should we do anything in the function implementations themselves to no-op in production?

Copy link
Contributor

@NullVoxPopuli NullVoxPopuli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 looks good to me.

I don't really know what to do about production debugging, because it is handy to have a good debugging experience in production -- which we already have -- I wonder if that would be more of an opt-out in the future to have all the meta and stuff stripped in prod.

Copy link
Contributor

@NullVoxPopuli NullVoxPopuli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approving, as this is mostly moving code out of the inspector

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants