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

Revamp components inspection (Octane support and improved UX) #1088

Merged
merged 34 commits into from
Dec 19, 2019

Conversation

chancancode
Copy link
Member

@chancancode chancancode commented Nov 20, 2019

Known Issues

  • Tests are failing
  • Should not show internal -top-level route template in "Components" tab
  • Right click on the page and selecting "Inspect Ember Component" shows the tooltip but does not select the corresponding row in the "Components" tab
    • Scroll to the appropriate row

Instructions for testing

  1. Click on the Checks tab for this pull request

  2. Under "Build and Publish" > "Build extensions" > "Artifacts", download the "chrome" build
    Screen Shot 2019-11-22 at 2 39 26 PM

  3. Extract the zip file you downloaded from the previous step, it should result in a folder called "chrome"

  4. Go to chrome://extensions/, enable "Developer mode" on the top-right corner, disable the Ember Inspector extension using the toggle (if you have it installed), click "Load unpacked", select the folder you extracted from the previous step
    Screen Shot 2019-11-22 at 2 41 08 PM

  5. Try out the new inspector
    inspect-5

  6. When you are done testing, go to chrome://extensions/, remove the custom build and switch back to the normal inspector

The custom extension installed using this method does not automatically update itself, so you will have to repeat these steps whenever there is a new commit available for testing.

@wycats wycats force-pushed the capture-render-tree branch 10 times, most recently from 0e9162d to c3ac9c2 Compare November 22, 2019 23:29
@patricklx
Copy link
Collaborator

I just tried this out 👍
one issue i saw

  • when a component updates the view (e.g checkbox component), the selected component (any, does not need to be the same component) in the inspector will be unselected... even though the instance (ember id) is the same

@chancancode
Copy link
Member Author

@patricklx I just pushed a refactor with various improvements, can you test it again and see if it is still a problem?

@joukevandermaas
Copy link

For other people who wanted to try it in Firefox instead of Chrome, follow these steps:

  1. Click on the Checks tab for this pull request
  2. Under "Build and Publish" > "Build extensions" > "Artifacts", download the "firefox" build.
  3. Extract the zip file you downloaded from the previous step, it should result in a folder called "firefox"
  4. Open the addons manager in Firefox (Ctrl+Shift+A on Windows, not sure about other OS)
  5. Click the little cog icon and select "Debug addons"
  6. In the new page that opens, click "Load temporary addon"
  7. Navigate to the directory you extracted in step 3, and select the "manifest.json" file
  8. When you are done testing, you can click the "remove" button on the temporary addon.

@nummi
Copy link
Collaborator

nummi commented Dec 13, 2019

Personal project running Ember Source 3.15.0 — I'm seeing a lot of this:

image

Three errors and [object object] for the component name.

Object Inspector error for bounds Error: You attempted to access the 'bounds' property on a Glimmer Component, but that property does not exist in Ember.js applications, it only exists in Glimmer.js apps

@chancancode
Copy link
Member Author

The last commit should fix the Glimmer Components debug assertion issue!

@wycats wycats force-pushed the capture-render-tree branch 2 times, most recently from b58c33d to 0fbca9f Compare December 17, 2019 21:16
@chancancode chancancode changed the base branch from master to refactor-acceptance-tests December 18, 2019 01:15
@chancancode chancancode changed the base branch from refactor-acceptance-tests to master December 18, 2019 02:02
@chancancode chancancode force-pushed the capture-render-tree branch 4 times, most recently from 08e4af6 to 3ecd5f0 Compare December 19, 2019 09:56
Comment on lines +5 to +7
let id = `ember-inspector-boot-${(Math.random() * 100000000).toFixed(0)}`;

let script = document.createElement('script');
Copy link
Member

Choose a reason for hiding this comment

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

I'm curious if these let's pollute "global space" for the consumer, have you tested?

Copy link
Member Author

Choose a reason for hiding this comment

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

It does not, it's in its own special scope (which is why I can't just set window.EmberENV = { _DEBUG_RENDER_TREE: true }; here in the first place)

Comment on lines 74 to 79
if (typeof factory.meta === 'object') {
factory.meta.moduleName = `my-app/${name}.hbs`;
factory.meta.moduleName = `my-app/templates/${name}.hbs`;
} else if (typeof factory.__meta === 'object') {
// Ember 3.13+
factory.__meta.moduleName = `my-app/${name}.hbs`;
factory.__meta.moduleName = `my-app/templates/${name}.hbs`;
}
Copy link
Member

Choose a reason for hiding this comment

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

It isn't immediately obvious to me why we actually care about setting meta.moduleName, what is it used for in the UI?

Copy link
Member Author

Choose a reason for hiding this comment

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

We use it to show the template name/path in the tooltips

Comment on lines 357 to 365
matchTree(tree, [TopLevel(
Route({ name: 'application' },
Route({ name: 'simple' },
Component({ name: 'test-foo', bounds: 'single' }),
Component({ name: 'test-bar', bounds: 'range' })
)
)
)]);
});
Copy link
Member

Choose a reason for hiding this comment

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

This abstraction is really nice 👏

@chancancode chancancode marked this pull request as ready for review December 19, 2019 18:55
@chancancode chancancode changed the title Capture render tree Revamp components inspection (Octane support and improved UX) Dec 19, 2019
@chancancode chancancode merged commit 4713877 into master Dec 19, 2019
@delete-merged-branch delete-merged-branch bot deleted the capture-render-tree branch December 19, 2019 20:51
nummi pushed a commit to nummi/ember-inspector that referenced this pull request Apr 1, 2020
…s#1088)

* Add a polyfill for the `captureRenderTree` API

* GlimmerTree -> RenderTree

* [WIP] view inspecting

* Correctly insert outlet nodes

* Wire-up more of the UI

* Get pinning working

* Fix vertical-collection

* Fix object inspection

* Fix polyfill

* Tooltip positionining

Closes emberjs#1089

* Use polyfill for 3.14.0/3.14.1

* Set `EmberENV._DEBUG_RENDER_TREE`

* Some improvements to the tooltip

* Refactor render tree to use Octane idioms

* Fix debouncing, don't cause a ton of user-runloops

* Don't shadow methods (requested by @rwjblue)

* Implement remaining features

* Automatically scroll to the appropiate row
* Make sure the view inspection tooltips and the inspector tab stays in
  sync

* Fix pinning

* Some performance improvements

* General cleanup, ensure correct teardown

* Revert to updating component tree with instrumentation

While the backburner-based approach is technically more correct, it
turns out that ember in practice always fire the instrumentation
events due to the route templates (including the internal -top-level)
being instrumented by default. This gives more favorable results since
we start way too many runloops when we shouldn't have to.

* Don't show empty rects

* Fix app-picker tests

* remove unused code

* Fix component tests

* Fix view debug tests

* Fix style lint

* PR comments

* Fix 3.4 outlet template location

* Fix @model arg on 3.14

* Skip debounce when requesting the first render tree

* unify view debug and component tree tests

* Refactor render-tree

* Refactor view-inspection
nummi pushed a commit to nummi/ember-inspector that referenced this pull request Apr 5, 2020
…s#1088)

* Add a polyfill for the `captureRenderTree` API

* GlimmerTree -> RenderTree

* [WIP] view inspecting

* Correctly insert outlet nodes

* Wire-up more of the UI

* Get pinning working

* Fix vertical-collection

* Fix object inspection

* Fix polyfill

* Tooltip positionining

Closes emberjs#1089

* Use polyfill for 3.14.0/3.14.1

* Set `EmberENV._DEBUG_RENDER_TREE`

* Some improvements to the tooltip

* Refactor render tree to use Octane idioms

* Fix debouncing, don't cause a ton of user-runloops

* Don't shadow methods (requested by @rwjblue)

* Implement remaining features

* Automatically scroll to the appropiate row
* Make sure the view inspection tooltips and the inspector tab stays in
  sync

* Fix pinning

* Some performance improvements

* General cleanup, ensure correct teardown

* Revert to updating component tree with instrumentation

While the backburner-based approach is technically more correct, it
turns out that ember in practice always fire the instrumentation
events due to the route templates (including the internal -top-level)
being instrumented by default. This gives more favorable results since
we start way too many runloops when we shouldn't have to.

* Don't show empty rects

* Fix app-picker tests

* remove unused code

* Fix component tests

* Fix view debug tests

* Fix style lint

* PR comments

* Fix 3.4 outlet template location

* Fix @model arg on 3.14

* Skip debounce when requesting the first render tree

* unify view debug and component tree tests

* Refactor render-tree

* Refactor view-inspection
patricklx pushed a commit to patricklx/ember-inspector that referenced this pull request Sep 19, 2022
…s#1088)

* Add a polyfill for the `captureRenderTree` API

* GlimmerTree -> RenderTree

* [WIP] view inspecting

* Correctly insert outlet nodes

* Wire-up more of the UI

* Get pinning working

* Fix vertical-collection

* Fix object inspection

* Fix polyfill

* Tooltip positionining

Closes emberjs#1089

* Use polyfill for 3.14.0/3.14.1

* Set `EmberENV._DEBUG_RENDER_TREE`

* Some improvements to the tooltip

* Refactor render tree to use Octane idioms

* Fix debouncing, don't cause a ton of user-runloops

* Don't shadow methods (requested by @rwjblue)

* Implement remaining features

* Automatically scroll to the appropiate row
* Make sure the view inspection tooltips and the inspector tab stays in
  sync

* Fix pinning

* Some performance improvements

* General cleanup, ensure correct teardown

* Revert to updating component tree with instrumentation

While the backburner-based approach is technically more correct, it
turns out that ember in practice always fire the instrumentation
events due to the route templates (including the internal -top-level)
being instrumented by default. This gives more favorable results since
we start way too many runloops when we shouldn't have to.

* Don't show empty rects

* Fix app-picker tests

* remove unused code

* Fix component tests

* Fix view debug tests

* Fix style lint

* PR comments

* Fix 3.4 outlet template location

* Fix @model arg on 3.14

* Skip debounce when requesting the first render tree

* unify view debug and component tree tests

* Refactor render-tree

* Refactor view-inspection
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants