fix(Scope): $broadcast and $emit should set event.currentScope to null #7523
Conversation
|
Thanks for the PR! Please check the items below to help us merge this faster. See the contributing docs for more information.
If you need to make changes to your pull request, you can update the commit with Thanks again for your help! |
When a event is finished propagating through Scope hierarchy the event's `currentScope` property should be reset to `null` to avoid accidental use of this property in asynchronous event handlers. In the previous code, the event's property would contain a reference to the last Scope instance that was visited during the traversal, which is unlikely what the code trying to grab scope reference expects. BREAKING CHANGE: $broadcast and $emit will now reset the `currentScope` property of the event to null once the event finished propagating. If any code depends on asynchronously accessing thei `currentScope` property, it should be migrated to use `targetScope` instead. All of these cases should be considered programming bugs. Closes angular#7445
|
I think this should be paired with a docs fix about not expecting the event object to look the same in a different turn of the event loop. Just a minor note about that |
| eventFired = true; | ||
| }); | ||
| grandChild.$emit('myEvent'); | ||
| expect(eventFired).toBeDefined(); |
There was a problem hiding this comment.
it's defined to begin with, should be toBe('true')
There was a problem hiding this comment.
good catch. I was refactoring old test and left in the old assertion.
fixed.
|
Looks okay to me with those minor nits addressed, and a note in the docs added regarding expectations of asynchronous behaviour |
|
👍 for a doc addition. Glad to be helpful. It was a pleasure to discover AngularJS under the hood (and it looks great!). |
|
landed. thanks @ncuillery and @caitp! |
When a event is finished propagating through Scope hierarchy the event's `currentScope` property should be reset to `null` to avoid accidental use of this property in asynchronous event handlers. In the previous code, the event's property would contain a reference to the last Scope instance that was visited during the traversal, which is unlikely what the code trying to grab scope reference expects. BREAKING CHANGE: $broadcast and $emit will now reset the `currentScope` property of the event to null once the event finished propagating. If any code depends on asynchronously accessing thei `currentScope` property, it should be migrated to use `targetScope` instead. All of these cases should be considered programming bugs. Closes angular#7445 Closes angular#7523
When a event is finished propagating through Scope hierarchy the event's
currentScopepropertyshould be reset to
nullto avoid accidental use of this property in asynchronous event handlers.In the previous code, the event's property would contain a reference to the last Scope instance that
was visited during the traversal, which is unlikely what the code trying to grab scope reference expects.
BREAKING CHANGE: $broadcast and $emit will now reset the
currentScopeproperty of the event tonull once the event finished propagating. If any code depends on asynchronously accessing thei
currentScopeproperty, it should be migrated to usetargetScopeinstead. All of these casesshould be considered programming bugs.
Closes #7445"