RangeError: Maximum call stack size exceeded

[teo-sk]

Angular version: 1.4.7
angular-bind-notifier version: 1.1.4 (also on 1.1.3 and master)

I'm currently optimizing a huge table with many nested ng-repeats which means the number of watchers grows exponentially with amount of data. Recently I changed all bindings to bind once, and used your cool little library to be able to refresh data in table and rebind one-time bindings when necessary (e.g. sorting).
Everything works fine unless the table is really huge (i.e. hundreds of rows), then I get this error:

RangeError: Maximum call stack size exceeded
    at wrap (http://localhost:5000/static/js/vendors/angular/angular-bind-notifier.js:20:46)
    at wrap (http://localhost:5000/static/js/vendors/angular/angular-bind-notifier.js:24:13)
    at wrap (http://localhost:5000/static/js/vendors/angular/angular-bind-notifier.js:24:13)
    at wrap (http://localhost:5000/static/js/vendors/angular/angular-bind-notifier.js:24:13)
    at wrap (http://localhost:5000/static/js/vendors/angular/angular-bind-notifier.js:24:13)
    at wrap (http://localhost:5000/static/js/vendors/angular/angular-bind-notifier.js:24:13)
    at wrap (http://localhost:5000/static/js/vendors/angular/angular-bind-notifier.js:24:13)
    at wrap (http://localhost:5000/static/js/vendors/angular/angular-bind-notifier.js:24:13)
    at wrap (http://localhost:5000/static/js/vendors/angular/angular-bind-notifier.js:24:13)
    at wrap (http://localhost:5000/static/js/vendors/angular/angular-bind-notifier.js:24:13) <span ng-bind=":refreshTable:data | niceNumber" class="ng-binding">

The table still contains several watchers per row, which I'm going to further optimize, so it's possible this issue won't appear when the watcher number will drop to 1-2 per row.

[kasperlewau]

Hey! I'm away for a couple of days so I won't be able to do a deep investigation into the issue until Wednesday I'm afraid.

Would you mind killing the piped filter in your ng-repeat binding to see if that's what causing the overflow?

I'd really appreciate a plunker or something similar as well.

Hang in there! Will look into this to the best of my possibility on Wednesday.

[teo-sk]

Thanks for the quick response!

Nope, killing the piped filter didn't help the problem.
I will try to replicate it in a plunker and post it here.

[teo-sk]

Hi, sorry, I just managed to create the plunker now.
Here it is: http://plnkr.co/edit/796YnA6G0WRgbzv2lV7C?p=info

Warning, running the plunker as-is will cause the browser to freeze, and will cause the error this issue is about. If you reduce the numbers in data generation method (eg. just one table), it will work just fine. Hope this helps to re-create and fix the issue!
Thanks

[kasperlewau]

Hey!

Sorry for being away, got home and instantly got a man-cold. So I've been pretty much dead in the water for a couple of days.

Cheers for the plunker - I'll have to dig deep here to try and figure out what the exact issue is. Any and all help in that regard would be greatly appreciated.

For now, I do not have a workaround that would solve your issue I'm afraid.

[kasperlewau]

Switching out ng-bind for a regular {{:refresh:column}} appears to have fixed the issue on my end.

new plunker

It's really really slow - but I think if you used a documentFragment or something of the sort you could reduce the performance loss of rendering all of these elements. Micro optimisations of the for loops might help you a bit here as well.

Oh and I bumped up the number of elements on the Plunker to 250.000. 😄

---

Eventually (when I get some time over) I'll dig deeper into what happens behind the scenes in the case of ng-bind and try to ship a fix soon™.

@teo-sk

[teo-sk]

Thank you very much for your suggestion, for now it's an acceptable workaround in my project. However, let's keep this issue open for you or some other contributor to track progress about the ng-bind usage, shall we?

[kasperlewau]

However, let's keep this issue open for you or some other contributor to track progress about the ng-bind usage

Sounds like a plan! Perhaps even reword the issue description and/or edit the main post to reflect that it has to do with ng-bind (among others I fear)?

[karlcassar]

I'm having the same exact issue, when I have loads of data on a page. I was delving deeply into the code of AngularJS and bind-notifier to try to see what is happening. The issue seems to be from when AngularJS caches parsed expressions, and the same expression is called twice.

In angular-bind-notifier (bindNotifier.js:128-129) you have the below code:

 expression = parse.call(this, '::' + rawExpression, interceptor);
 expression.$$watchDelegate = dynamicWatcher(expression, notifiers);

This calls the $parse() function of AngularJS, method copied below:

return function $parse(exp, interceptorFn, expensiveChecks) {
  var parsedExpression, oneTime, cacheKey;

  switch (typeof exp) {
    case 'string':
      exp = exp.trim();
      cacheKey = exp;

      var cache = (expensiveChecks ? cacheExpensive : cacheDefault);
      parsedExpression = cache[cacheKey]; // ### THIS IS THE ISSUE  ### 

      if (!parsedExpression) {
        if (exp.charAt(0) === ':' && exp.charAt(1) === ':') {
          oneTime = true;
          exp = exp.substring(2);
        }
        var parseOptions = expensiveChecks ? $parseOptionsExpensive : $parseOptions;
        var lexer = new Lexer(parseOptions);
        var parser = new Parser(lexer, $filter, parseOptions);
        parsedExpression = parser.parse(exp);
        if (parsedExpression.constant) {
          parsedExpression.$$watchDelegate = constantWatchDelegate;
        } else if (oneTime) {
          parsedExpression.$$watchDelegate = parsedExpression.literal ?
              oneTimeLiteralWatchDelegate : oneTimeWatchDelegate;
        } else if (parsedExpression.inputs) {
          parsedExpression.$$watchDelegate = inputsWatchDelegate;
        }
        cache[cacheKey] = parsedExpression;   // ### THIS IS THE ISSUE  ### 
      }
      return addInterceptor(parsedExpression, interceptorFn);

    case 'function':
      return addInterceptor(exp, interceptorFn);

    default:
      return noop;
  }
};

The angular code uses a cache mechanism, which will return the same expression as before when it encounters the same exact cacheKey. I could not determine why it is meeting the same cacheKey only in this instance and hence using the cache, but that is irrelevant. I temporarily disabled the cache in angularJS by commenting out the lines to confirm, and it worked perfectly. I've marked the lines I'm talking about above by the comment // ### THIS IS THE ISSUE ###.

When the cache is not hit, the returned expression.$$watchDelegate contains a oneTimeWatchDelegate or oneTimeLiteralWatchDelegate. If the cache is hit, the returned expression.$$watchDelegate will be a reference to your wrap() function, as the cached parsedExpression is the same one on which you are modifying the $$watchDelegate in your code.

Then, the dynamicWatcher() will we-wrap the $$watchDelegate into another wrap() call, and keeps on doing this, resulting in the maximum call stack size being reached when you have loads of elements.

I think a solution would be such that you should detect when parse.call() returns a cached expression, and do not we-wrap such an expression.

I am using Angular v1.4.8

[AntoineEsteve]

@karlcassar is right. The wrapper is wrapped again and again. The solution would be to flag the wrapper and return the watchDelegate if it is a wrapper.

  function dynamicWatcher (expr, notifierKeys) {
    function setupListeners (scope, cb) {
      notifierKeys.forEach(function (nk) {
        scope.$on('$$rebind::' + nk, cb);
      });
    }

    if (expr.$$watchDelegate.isWrapper)
      return expr.$$watchDelegate;

    function wrap (watchDelegate, scope, listener, objectEquality, parsedExpression) {
      var delegateCall = watchDelegate.bind(this, scope, listener, objectEquality, parsedExpression);

      /**
       * - Ensure that the watchDelegate has a prototype.
       * - Ensure that said prototype has a constructor property (in a cumbersome way).
       *
       * In doing so, we are certain that we do not increase the listener count exponentially. Black magic.
       */
      if (watchDelegate.prototype && Object.getOwnPropertyNames(watchDelegate.prototype).indexOf('constructor') > -1) {
        setupListeners(scope, delegateCall);
      }

      delegateCall();
    }

    var wrapper = wrap.bind(this, expr.$$watchDelegate);
    wrapper.isWrapper = true;
    return wrapper;
  }

[kasperlewau]

Thanks a lot for the assistance here @karlcassar @AntoineEsteve!

I'll get on this sometime tomorrow during my 5 hour layover at Heathrow (yuck). Unless of course you would like to post a PR with some accompanying unit tests. If you need any help with that, let me know 😄.

[kasperlewau]

@teo-sk @karlcassar @AntoineEsteve

I just pushed , 143bb5a which implements what Antoine suggested and it appears to working just fine™ on my end.

Would love to get some feedback from you guys (as I don't have all the test cases in front of me) before pushing the next release.

[karlcassar]

Hi @kasperlewau . Unfortunately I was super busy and missed out on this comment. Currently as a quick-workaround, I had removed the bind-notifier where it was giving issues. I've tried the update on my code and the watch count still exploded significantly when hiding/showing elements, so the issue is not completely resovled. Also, from that time, another component started giving the 'call stack size exceeded' issue as well. I'll try to create a failing unit test and let you know about it.

[kasperlewau]

@karlcassar Do note that the pushed code has not been released yet. If you would like to try it out, scrap any bower install and simply inject the following into your application;

<script src="https://rawgit.com/kasperlewau/angular-bind-notifier/master/dist/angular-bind-notifier.js"></script>

---

If you've already done that, then this needs some further investigation - though I cannot replicate it with the current master code in place 😢

[karlcassar]

@kasperlewau Yes, i had downloaded the latest update by specifying the version guid in bower - and I ensured that the .js had the updated code - so I definitely had your latest update. It's difficult to replicate it, but I'll try to create a failing unit test for it.

[karlcassar]

@kasperlewau , I managed to create a "failing test". However this is for the exploding watch count rather than the actually maximum call stack exceeded. I strongly believe though that this issue is related to the maximum call stack error as this will keep on increasing the watch count, until at one point the browser might throw the call stack exception.

Plunkr: http://plnkr.co/edit/WAHdlIyiog1gdiEwqHa4

Code is using your version of bind-notifier with the updated code (direct link).

Read the README.md of the Plunkr as I tried to explain better what is happening, and how to check it. I discovered this issue when I was trying to reduce greatly the watch count in an application, and I discovered that rather than decreasing, bind-notifier was increasing them.

[kasperlewau]

I have yet to be able to replicate either of the following (in a unit test environment, mind you);

a) Exploding watch count.
b) Call stack exceeded.

Having toyed around with the given plunker, I do see the watch count increasing when coupling a notifierExpression with ng-if. That's about as far as I've come in investigating the issue(s) at hand.

I sorely need to teach my body to survive on <6hrs of sleep 😐

[karlcassar]

Hi Kasper,

Yeah I know it's not that simple to just replicate in a unit test
environment. However, maybe one way to start would be to look into how
angular actually stores the watches, and see what are the duplicate /
exploding watches are, and when they are created? It definitely is related
to an ng-if.

Thanks for this! And goodluck on surviving on less than 6 hours sleep, I
tried and don't suggest it though! :)

Regards,

Karl

---

*mob: *+356 7979 2211

On 27 January 2016 at 10:14, Kasper Lewau notifications@github.com wrote:

I have yet to be able to replicate either of the following (in a unit test
environment, mind you);

a) Exploding watch count.
b) Call stack exceeded.

Having toyed around with the given plunker, I do see the watch count
increasing when coupling a notifierExpression with ng-if. That's about as
far as I've come in investigating the issue(s) at hand.

I sorely need to teach my body to survive on <6hrs of sleep [image:
😐]

—
Reply to this email directly or view it on GitHub
#23 (comment)
.