Skip to content
This repository has been archived by the owner on Apr 12, 2024. It is now read-only.

Removing watches from inside a watch can cause dirty watches to be skipped (regression) #5525

Closed
rino65 opened this issue Dec 24, 2013 · 3 comments

Comments

@rino65
Copy link

rino65 commented Dec 24, 2013

Due to recent changes that added optimization in the digest function with lastDirtyWatch
if(watch === lastDirtyWatch) {....}
#5287

The scenario
during digest , one watch was called that caused a second watch to be removed. The first watch had an index which was larger then the second watch in the watchers array so the remove shifted all the indexes to the left , and the next iteration when getting length--; watchers[length] it will return the previous watch that was already called and so the condition watch === lastDirtyWatch will be true and it breaks from the loop and miss calls to the remaining watches in the array (some which have also changed)

one option to solve this is to nullify lastDirtyWatch in the unregister function that is returned from $watch

    return function() {
      arrayRemove(array, watcher);
      lastDirtyWatch = null;
    };

other is to defer the removal of the watcher till after the digest loop .

@IgorMinar
Copy link
Contributor

wow.. nice report!

@IgorMinar
Copy link
Contributor

resetting the lastDirtyWatch sounds like a good enough approach to me. it's not ideal as it will cause this optimization to be disabled, but if we find that these scenarios are common enough, we can create an optimization that won't break on watch removal.

IgorMinar added a commit to IgorMinar/angular.js that referenced this issue Jan 2, 2014
@ryan65
Copy link

ryan65 commented Jan 6, 2014

Hey Igor,
That's just great
thanks

jamesdaily pushed a commit to jamesdaily/angular.js that referenced this issue Jan 27, 2014
jamesdaily pushed a commit to jamesdaily/angular.js that referenced this issue Jan 27, 2014
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants