Local scope.$digest can trigger $rootScope.$apply via a < binding

[samal-rasmussen]

Do you want to request a feature or report a bug?

bug

What is the current behavior?

A call to a local scope.$digest can trigger a < binding watcher. A < binding watcher will call $rootScope.$apply.

If the current behavior is a bug, please provide the steps to reproduce and if possible a minimal demo of the problem via https://plnkr.co or similar (template: http://plnkr.co/edit/tpl:yBpEi4).

The watcher for the < binding calls recordChanges(), which calls flushOnChangesQueue(), which calls rootScope.$apply. See: https://github.com/angular/angular.js/blob/master/src/ng/compile.js#L1516

What is the expected behavior?

I thought I could expect a local scope.$digest to only call its own watchers and those of its children. This would be a great performance gain in specific use cases.

What is the motivation / use case for changing the behavior?

Being able to have local scope digest cycles may be necessary for some large Angular applications, as rootscope digest might be too expensive, either because there are too many watchers in a large app, or because there are many push updates from the backend.

Which versions of Angular, and which browser / OS are affected by this issue? Did this work in previous versions of Angular? Please also test with the latest stable and snapshot (https://code.angularjs.org/snapshot/) versions.

This affects the latest commit to the file in question: https://github.com/angular/angular.js/blob/736b6c7fed79c8305786bbb86d39dd7af891a162/src/ng/compile.js

Other information (e.g. stacktraces, related issues, suggestions how to fix)

Maybe we should be able to configure per < binding whether or not we want the watcher to trigger a $rootScope.$apply or a scope.$digest.

[gkalpak]

The problem with this is that a local change can have a non-local impact. Local $digest is generally discouraged and changing this globally would lead to very cryptic bugs.

Making this configurable per binding would be an option, but it would (a) be too complicated to implement and (b) complicate the directive API quite a bit to make it worth it.

I will leave this open to hear form others, but it is 👎 from me.

[samal-rasmussen]

Well its nice to know I've understood the inner workings of Angular well enough to at least describe the issue I'm having correctly :)

I was just really surprised to see a dramatic jump in the amount of rootscope digest cycles, when I cloned an object to force a < binding to properly trigger. I've got that object bound two levels deep into a grandchild component, and so I'm actually seeing the rootscope digest firing trice, when it was only firing once before :~(

[DanielCaspers]

Hey @gkalpak, I know this is asking a lot, but could you spare a moment to run through a brief example (written, not necessarily in code) just to walk through how this plays out in the integration with browser event loop so @samal84 and I can more deeply understand an example of why the local change would have non local impact?

[gkalpak]

I might have something much simpler in mind than you think (nothing involving the browser event loop). Imagine this:

.component('myComponent', {
  bindings: {
    input: '<'
  },
  controller: function MyComponentController($rootScope, SomeService) {
    this.$onChanges = function(changes) {
      $rootScope.$emit('new input', changes.input.currentValue);
      // or
      SomeService.setValue(changes.input.currentValue);
    }
  }
})

In the above scenario, $rootScope.$emit(...) or SomeService.setValue(...) could be affecting any part of the application (e.g. another directive could react to these changes).

[samal-rasmussen]

Yeah yeah exactly. Angular can't know this, but the developer can. So what I was fishing for was a configuration option on the binding to say that I know that the $onChanges hook doesn't do any global changes, and that a local scope $digest would be fine.

[dcherman]

@gkalpak Do you happen to know why $onChanges is invoked in the $$postDigest queue anyway? The reason I ask is that unless I'm mistaken, the digest cycle uses pre-order traversal - tested that here to make sure I was correct: http://jsfiddle.net/rogqp47y/54/

With that in mind, any changes in the parent scope and subsequent one-way bindings should be correctly reflected by the time we've executed the bindings for any given scope; can we invoke $onChanges after executing all explicitly bound watchers but before continuing to the next iteration of the digest cycle? Not sure what the implications there might be at the moment.

[petebacondarwin]

@samal84 - you would need to be sure that not only does the $onChanges call not trigger any global changes but also that no watches that were triggered by the $digest did not do so either.

This includes that any transcluded content doesn't interact globally too. For example: http://plnkr.co/edit/OATpjxGhB5kBl9VNWoNz?p=preview

[petebacondarwin]

@dcherman - can you clarify what you are suggesting?

[dcherman]

@petebacondarwin What I was suggesting won't work since it breaks coalescing multiple changes of a value into one invocation of $onChanges. Did some testing a few minutes ago.

I was just suggesting that rather than executing the hook in a post digest call, we execute it immediately after the watchers for the associated scope, so it becomes a kind of specialized watchCollection. That won't work for the reason I gave above though as multiple changes would not be coalesced.

I do think that it may be valid to disregard the case of mutating global state or state external to your scope hierarchy though. When a developer invokes $digest explicitly on a scope rather than $apply, he/she is making an implicit promise that no content, transcluded or otherwise, will affect state outside of the subtree being digested. That's why it's a more advanced optimization since you need that knowledge.

It wouldn't be difficult to re-invoke the $digest on the correct scope where the $digest began rather than the $rootScope though, and that would at least alleviate this a little bit. I have the basic idea commited here, just considering any implications before opening a PR

[poshest]

+1

@dcherman how is the PR going? :)