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

[BUGFIX] Ensures removeAllListeners doesn't break subsequent adds #17184

Conversation

pzuraq
Copy link
Contributor

@pzuraq pzuraq commented Nov 6, 2018

This popped up as a bug in Ember Inspector because they use the REMOVE_ALL functionality during tests. REMOVE_ALL's are tricky, they don't neatly fall into the new model for listeners (which is why we are deprecating and removing them in 3.9) but for now, this should prevent that particular bug from occurring again.

Resolves #17183

@pzuraq
Copy link
Contributor Author

pzuraq commented Nov 6, 2018

cc @rwwagner90 @krisselden @rwjblue

Copy link
Member

@rwjblue rwjblue left a comment

Choose a reason for hiding this comment

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

Just confirming: this doesn't impact the performance characteristics?

@pzuraq
Copy link
Contributor Author

pzuraq commented Nov 6, 2018

This should not, REMOVE_ALL is very uncommon and when it's not a REMOVE_ALL those listeners will actually have the exact target and method, which is why the assignment won't be needed. The basic shape of the listeners will be the same in any case, though they may have slightly different types.

@rwjblue rwjblue merged commit 315fa08 into emberjs:master Nov 6, 2018
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.

3 participants