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 😢