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

Infinite $digest() loop AngularJS with filter expression on html attribute #15874

Closed
2 of 3 tasks
aredfox opened this issue Mar 31, 2017 · 23 comments
Closed
2 of 3 tasks

Comments

@aredfox
Copy link

aredfox commented Mar 31, 2017

I'm submitting a ...

  • bug report
  • feature request
  • other (Please do not submit support requests here (see above))

Current behavior:

When I create a filter expression on an html attribute, an attribute that is later bounded incoming in a component and used inside an ng-repeat, it triggers an infinite digest cycle/loop. The output comes up fine on the screen, but there are errors in the console suggesting that it triggers and infinite digest cycle. Code to reproduce is on a plnkr: http://plnkr.co/edit/JdiLEIyji2pHd3eeNMUL?p=preview.
This also happens when the array I'm passing is an array of literals such as number or string (http://plnkr.co/edit/mfjPgZjBro39Nb7WYpjU?p=preview).

Expected / new behavior:

  1. If it is a bug, then the behaviour without the bug would not result in an infinite digest cycle, and it would improve readability / seperation of concerns. As the component I'm passing in the filtered array only needs to take care of how it's showing the items in the array, not extra filter params.
  2. If it isn't a bug, then I feel documentation needs to be changed as now I feel the error message is unclear to me, and the docs now ways it is possible to use filter expressions inside those places..

Minimal reproduction of the problem with instructions:

Here's a plnkr with the code in action http://plnkr.co/edit/JdiLEIyji2pHd3eeNMUL?p=preview - open the devtools in your browser to see the errors appear.

Angular version: 1.5.* and 1.6.1/2/3

Browser: [all]

Anything else:

Error message

Error: [$rootScope:infdig] http://errors.angularjs.org/1.6.3/$rootScope/infdig?p0=10&p1=%5B%5B%7B%22ms…e%2C%22type%22%3A1%2C%22%24%24hashKey%22%3A%22object%3A5%22%7D%5D%7D%5D%5D
    at eval (angular.js:38)
    at m.$digest (angular.js:18048)
    at m.$apply (angular.js:18280)
    at eval (angular.js:1912)
    at Object.invoke (angular.js:5003)
    at c (angular.js:1910)
    at Object.Pc [as bootstrap] (angular.js:1930)
    at execute (VM877 main.ts!transpiled:21)
    at f (system.js:5)
    at Object.execute (system.js:5)

Possible solutions

  1. In the repo where I had the problem at first: I did <todo-list todo-items="$ctrl.todoItems" filter-by="{completed:true}"></todo-list>. For full source see here: aredfox/todo-angularjs-typescript@e71900b. But I feel it's working around the problem, plus I don't understand why this triggers a $digest() cycle and why it shouldn't just work.

Related issue

  1. StackOverflow post of this issue: http://stackoverflow.com/questions/43119977/infinite-digest-loop-angularjs-with-filter-expression-on-html-attribute
@frederikprijck
Copy link
Contributor

frederikprijck commented Mar 31, 2017

FWIW: I've edited your plunkr to reproduce the issue with an easier filter syntax. I guess it doesn't matter, but I wanted to drop it here just to be sure:
https://plnkr.co/edit/mnamqTtsogXZj8inB3L5?p=preview

plus I don't understand why this triggers a $digest() cycle and why it shouldn't just work.

A $digest cycle get's triggered whenever something changes (and angularjs is aware of these changes), filtering your array results in a new array hence a changed input binding and a need for the digest cycle to trigger in order for angular to know what has to be updated and what does not.

@Narretz
Copy link
Contributor

Narretz commented Mar 31, 2017

The issue with filtered arrays in component bindings is discussed here #14039
Implementing $doCheck is recommended, but I don't actually know how you would do that. :o

@frederikprijck
Copy link
Contributor

frederikprijck commented Mar 31, 2017

I'm interested to see how to implement a workaround using $doCheck, as mentioned in the issue you linked!

@petebacondarwin
Copy link
Contributor

One quick workaround is to use =* binding rather than < since the former will not continue to trigger digests. See https://plnkr.co/edit/fk9SXfRQa3cE8KGfGWqt?p=preview

@frederikprijck
Copy link
Contributor

Yep, that did solve the plunkr I added: https://plnkr.co/edit/4czmVoTGz1YvM63IrPqs?p=preview

Out of curiosity: Anything available regarding the $doCheck you mentioned @petebacondarwin ?

@petebacondarwin
Copy link
Contributor

Thinking about it now :-)

@petebacondarwin
Copy link
Contributor

I think this is actually a bug - we are getting stuck in a state where it is testing ['a'] === ['a'] which is continually false.

@gkalpak
Copy link
Member

gkalpak commented Mar 31, 2017

Isn't this intended behavior?

@frederikprijck
Copy link
Contributor

frederikprijck commented Mar 31, 2017

If this is intended, I think it's odd to see the UI work but the console error out.

@gkalpak
Copy link
Member

gkalpak commented Mar 31, 2017

This is also expected behavior when there is an infinite digest error. We exit the digest loop to avoid freesing the browser. The error doesn't mean that there is something wrong with your app, except that some binding/watcher is constantly changing. In this case, the change doesn't matter, since the two array are identical (even if not pointing to the same reference), so it is expected that your app continues to work as expected.

@petebacondarwin
Copy link
Contributor

But I don't think we should hit an infinite loop here. Why is the array identity changing?

@Narretz
Copy link
Contributor

Narretz commented Mar 31, 2017

Because of the filter, no?

@petebacondarwin
Copy link
Contributor

OK, so the filter always returns a new object and the one-way binding only tests for object identity equivalence. So it is expected behaviour.

Here is a reasonable workaround (IMO), which could be tailored to the specific needs of the situation for best performance: https://plnkr.co/edit/g0AzLUAbwfiiIl8PNgcE?p=preview

<comp items="$ctrl.items | filter: $ctrl.filterBy | stable"></comp>
  .filter('stable', function() {
    var oldValue = NaN;
    return function(value) {
      oldValue = angular.equals(value, oldValue) ? oldValue : value;
      return oldValue;
    };

We should document this.

@gkalpak
Copy link
Member

gkalpak commented Mar 31, 2017

I am pretty sure this will break if you have more than one | stable in your template.

@aredfox
Copy link
Author

aredfox commented Mar 31, 2017

Thanks, @petebacondarwin, would using =* not be a viable option too? Or are there huge perf issues involved? Because this setup, it works of course, does make it a rather big workaround imho. Is there a possibility of ever having a <* binding?

@petebacondarwin
Copy link
Contributor

@gkalpak - for sure, it would need tweaking for production :-)

@aredfox - There is nothing wrong with using =* as long as you don't want to ensure that changes inside the component do not appear outside the component.

I guess we are getting close to enough use cases to implement <*

@Narretz
Copy link
Contributor

Narretz commented Mar 31, 2017

@jbedard previously wondered if this could be handled in $parse directly: #14039 (comment)

@jbedard
Copy link
Contributor

jbedard commented May 9, 2018

This should also be fixed by #16553 which adds one-way collection watching (<*) similar to the bidirectional version (=*).

@wilker7ribeiro
Copy link

wilker7ribeiro commented Jun 14, 2018

Would be nice if we have one-time bindings in component and directive definition too, like "::<" or "<::" like we can use inside templates {{::$ctrl.variable}}

@gkalpak
Copy link
Member

gkalpak commented Jun 15, 2018

That would be cool indeed 😃

Right now, only the user of the component/directive has control over whether the binding is normal or one-time (which makes sense since they know whether the value can change). But it would be cool if the component/directive author could also have control, because oftentimes components/directives do to react to binding value changes, so there is no point in keeping the watcher active (but the user doesn't have to know about this).

@wilker7ribeiro, could you create a separate issue about this.
(We are aiming to have a feature freeze before the end of the month (as AngularJS is entering its LTS period), so we can't promise anything 😉)

@petebacondarwin
Copy link
Contributor

petebacondarwin commented Jun 20, 2018

Two ways you could do this today:

link: function(scope, element, attr) {
  var oneTimeValue = $parse(attr.oneTimeAttr)(scope.$parent);
});
link: function(scope, element, attr) {
  var oneTimeValue = undefined;
  var watchRemover = scope.$parent.$watch(attr.oneTimeAttr, function(newValue, oldValue) {
    if (newValue !== undefined) {
      oneTimeValue = newValue;
      watchRemover();
  });
});

@petebacondarwin
Copy link
Contributor

Duplicate of #16173, right?

@gkalpak
Copy link
Member

gkalpak commented Jun 21, 2018

Indeed. And I'm afraid I'll have to agree with myself 😁

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.