watchCollection: Return correct oldCollection argument to listener #5661
Conversation
|
I'm sorry, but I wasn't able to verify your CLA signature. CLA signature is required for any code contributions to AngularJS. Please sign our CLA and ensure that the CLA signature email address and the email address in this PR's commits match. If you signed the CLA as a corporation, please let me know the company's name. Thanks a bunch! PS: If you signed the CLA in the past then most likely the email addresses don't match. Please sign the CLA again or update the email address in the commit of this PR. |
|
CLA signature verified! Thank you! Someone from the team will now triage your PR and it will be processed based on the determined priority (doc updates and fixes with tests are prioritized over other changes). |
ghost
assigned …er - include a test.
|
@IgorMinar - this issue has been raised in several ways and various fixes proposed. Any chance that it will be resolved any time soon? |
|
|
||
| $rootScope.obj = undefined; | ||
| $rootScope.$digest(); | ||
| expect(paramsEqual).toBeFalsy(); |
There was a problem hiding this comment.
please don't use toBeFalsy or toBeTruthy in angular test suite. We prefer strict checks like toBe(true) or toBeUndefined() instead
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
|
@johnsoftek I pounded on this a big yesterday and today and came up with this: #6736 let me know what you think. |
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
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
Fixes #2621.
At present, when watchCollection calls a registered listener, the oldCollection argument has the same value as
the newCollection argument. This does not accord with the API description: "The newCollection object is the newly modified data obtained from the obj expression and the oldCollection object is a copy of the former collection data.".
This fix caches the collection value correctly. It also includes some optimisations to avoid copy activity when the collection is unchanged.