This repository has been archived by the owner on Apr 12, 2024. It is now read-only.
fix(rootScope): $watchCollection returns in oldCollection a copy of the ... #5688
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…he former array data related to angular#1751 When watching arrays, $watchCollection returned the new data both in the newCollection and the oldCollection arguments of the listener
This was referenced Jan 8, 2014
ghost
assigned 
| @@ -439,6 +441,8 @@ function $RootScopeProvider(){ | |||
| changeDetected++; | |||
| } | |||
|
|
|||
| oldArray = oldValue.length > 0 ? Array.prototype.slice.call(oldValue, 0) : []; | |||
There was a problem hiding this comment.
this should be:
if (listener.length > 1) {
oldArray = slice(oldValue, 0);
}
or something along those lines. the important bit is listener.length - we don't want to compute this value if the listener is not asking for it. (we'll break the listener if it uses the arguments object to get hold of args, but I think it's a fair trade-off for hot code like this.
There was a problem hiding this comment.
we also have a slice fn defined in Angular.js file, so you should use it
|
Another implementation + related tweaks here: #5661 |
IgorMinar
added a commit
to IgorMinar/angular.js
that referenced
this pull request
Mar 18, 2014
Originally we destroyed the oldValue by incrementaly copying over portions of the newValue into the oldValue during dirty-checking, this resulted in oldValue to be equal to newValue by the time we called the watchCollection listener. The fix creates a copy of the newValue each time a change is detected and then uses that copy *the next time* a change is detected. To make `$watchCollection` behave the same way as `$watch`, during the first iteration the listener is called with newValue and oldValue being identical. Since many of the corner-cases are already covered by existing tests, I refactored the test logging to include oldValue and made the tests more readable. Closes angular#2621 Closes angular#5661 Closes angular#5688
|
I have an alternative implementation, please take a look: #6736 |
IgorMinar
added a commit
to IgorMinar/angular.js
that referenced
this pull request
Mar 18, 2014
Originally we destroyed the oldValue by incrementaly copying over portions of the newValue into the oldValue during dirty-checking, this resulted in oldValue to be equal to newValue by the time we called the watchCollection listener. The fix creates a copy of the newValue each time a change is detected and then uses that copy *the next time* a change is detected. To make `$watchCollection` behave the same way as `$watch`, during the first iteration the listener is called with newValue and oldValue being identical. Since many of the corner-cases are already covered by existing tests, I refactored the test logging to include oldValue and made the tests more readable. Closes angular#2621 Closes angular#5661 Closes angular#5688 Closes angular#6736
IgorMinar
added a commit
that referenced
this pull request
Mar 18, 2014
Originally we destroyed the oldValue by incrementaly copying over portions of the newValue into the oldValue during dirty-checking, this resulted in oldValue to be equal to newValue by the time we called the watchCollection listener. The fix creates a copy of the newValue each time a change is detected and then uses that copy *the next time* a change is detected. To make `$watchCollection` behave the same way as `$watch`, during the first iteration the listener is called with newValue and oldValue being identical. Since many of the corner-cases are already covered by existing tests, I refactored the test logging to include oldValue and made the tests more readable. Closes #2621 Closes #5661 Closes #5688 Closes #6736
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
...former array data
related to #1751
When watching arrays, $watchCollection returned the new data both in the newCollection
and the oldCollection arguments of the listener
Split from PR #3865