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

Ensure renderDebug is properly unregistered. #1202

Merged
merged 1 commit into from
May 12, 2020

Conversation

rwjblue
Copy link
Member

@rwjblue rwjblue commented May 12, 2020

Adds a side effecting import to tests/test-helper.js to ensure that ember-debug/main can be imported/required eagerly in the test suite (and is not broken by a cycle).

Updates all lazy requires for ember-debug modules to ES module import statements.

@@ -29,7 +29,7 @@ export default EmberObject.extend(PortMixin, {
};

this.profileManager.offProfilesAdded(this, this.sendAdded);
this.profileManager.offProfilesAdded(this, this._updateRenderPerformanceAndComponentTree);
Copy link
Member Author

Choose a reason for hiding this comment

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

This method doesn't exist on ember-debug/render-debug at all. It was removed in 4713877#diff-04f0316e4c3ec9f79f8bf1ec48f6aafbL45, but we missed updating the cleanup code with the renamed method.

Adds a side effecting import to `tests/test-helper.js` to ensure that
`ember-debug/main` can be imported/required eagerly in the test suite
(and is not broken by a cycle).

Updates all lazy requires for `ember-debug` modules to ES module import
statements.
@rwjblue rwjblue force-pushed the ensure-cleanup-on-destruction branch from e7ba7cb to ec3b023 Compare May 12, 2020 02:29
@chancancode
Copy link
Member

Looks good to me but is it possible to set up a CI scenario where we run the app test only, to avoid accidentally relying on ember debug existing?

@chancancode
Copy link
Member

(ember-try tests are ember debug only, so they are like the opposite of that, and this new scenario can go into the same “phase”)

@RobbieTheWagner RobbieTheWagner merged commit c9d8359 into master May 12, 2020
@delete-merged-branch delete-merged-branch bot deleted the ensure-cleanup-on-destruction branch May 12, 2020 12:29
patricklx pushed a commit to patricklx/ember-inspector that referenced this pull request Sep 19, 2022
Adds a side effecting import to `tests/test-helper.js` to ensure that
`ember-debug/main` can be imported/required eagerly in the test suite
(and is not broken by a cycle).

Updates all lazy requires for `ember-debug` modules to ES module import
statements.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants