perf(Scope): limit propagation of $broadcast #5371
Conversation
ghost
assigned 
|
I really like the fact that awareness of the issue seems to grow among angular folks. And I do think this is a great addition to make Users should then bind through I would still vote for angular to provide such an Because the problem is, let's say you have a directive (deeply nested in scopes) which is used plenty of times on your site (e.g. in a repeater) which binds via Just my two cents ;-) |
|
To be fair, it will only walk down the branches of the tree needed to get to your deeply nestled scope, but will not pass through the sibling branches on the way. |
| @@ -998,8 +1020,7 @@ function $RootScopeProvider(){ | |||
| listeners, i, length; | |||
|
|
|||
| //down while you can, then up and next sibling or up and next sibling until back at root | |||
| do { | |||
| current = next; | |||
| while ((current = next)) { | |||
There was a problem hiding this comment.
which change do-while to while? it doesn't seem to make a difference and it causes the code to go out of sync with the traversal code in digest
There was a problem hiding this comment.
It struck me as redundant and weird:
do {
current = next;
...
} while ((current = next));
is identical to (or perhaps a very tiny bit slower than) the simpler:
while ((current = next)) {
...
}
There was a problem hiding this comment.
And for that matter, they were already out of sync in this respect. The similar loop in $digest is:
do {
// current = next is not here.
} while ((current = next))
|
otherwise this looks good. I do like what @cburgdorf suggested and I triaged #4574 for 1.3 |
This adds an $onRootScope method to the scope type. This allows developers to use $rootScope.$emit + $scope.$onRootScope as a fast eventBus which doesn't use scope bubbleing. Fixes angular#4574, Relates to angular#5371
| } | ||
| current.$$listenerCount[name]++; | ||
| } while ((current = current.$parent)); | ||
|
|
||
| return function() { | ||
| namedListeners[indexOf(namedListeners, listener)] = null; |
There was a problem hiding this comment.
you need to decrement the counters here as well
|
it would be better to refactor this by creating a single function that adjusts the counters and then use this fn to increment and decrement counters in all 3 places instead of repeating the same code everywhere |
| spy = jasmine.createSpy('listener'), | ||
| firstSecond = first.$new(); | ||
|
|
||
| firstSecond.$on(EVENT, spy); |
There was a problem hiding this comment.
please register two listeners for this event so that we test that we decrement the counters by the right amount (not just by one)
…stered the event Update $on and $destroy to maintain a count of event keys registered for each scope and its children. $broadcast will not descend past a node that has a count of 0/undefined for the $broadcasted event key. Resolves angular#5341
|
@cburgdorf I am perhaps misunderstanding. When you say:
I don't see this as being true at all. The documentation clearly distinguishes I realize that this landed and the change is great. I'm just adding this to make it clear that |
|
It's indeed a misunderstanding. What I am saying is |
|
Cool, thanks for clarifying. |
…eners for the event Update $on and $destroy to maintain a count of event keys registered for each scope and its children. $broadcast will not descend past a node that has a count of 0/undefined for the $broadcasted event key. Closes angular#5341 Closes angular#5371
…eners for the event Update $on and $destroy to maintain a count of event keys registered for each scope and its children. $broadcast will not descend past a node that has a count of 0/undefined for the $broadcasted event key. Closes angular#5341 Closes angular#5371
perf(Scope): limit propagation of $broadcast to scopes that have registered the event
Update $on and $destroy to maintain a count of event keys registered for each scope and its children.
$broadcast will not descend past a node that has a count of 0/undefined for the $broadcasted event key.
Resolves #5341