ng-repeat-start error with angular 1.5.0

[kimx]

Hello
I have an empty array that will use ng-repeat-start to render HTML.But I got some error when click button to add item to array. As below photo

ps:

I've tried kind of various to find out.Finally I found below code will cause this error.

app.directive("ngRepeat", ['$animate', function ($animate) {
return function ($scope, $element) {
$animate.enabled($element, false);
};
}]);

At beginning I thought it caused by $animate.enabled. after I empty the function as below code that still error.

app.directive("ngRepeat", [ function () {
return function ($scope, $element) {
};
}]);

Ps:1.This problem just occurs when I upgrade to 1.5.0
2.I use chrome 64 bit.This error will not occurs on IE 11 or IE Edge

simple example
plnkr

Please help the Issue,thank you very much.

[kimx]

I try to remove some module js e.g. angular-route.min.js angular-messages.min.jsangular-sanitize.min.js.Then the error will still happen but frequency is decreased.

[Narretz]

I can't reproduce this error in your plnkr. The plnkr throws a different error:

Error: [ngRepeat:dupes] http://errors.angularjs.org/1.5.0/ngRepeat/dupes?p0=product%20in%20Products&p1=string%3AiPhone6s&p2=iPhone6s

because you are pushing the same element to your Products array. Here's a plnkr with that problem fixed: http://plnkr.co/edit/ik0xHn9sbjpZ51J4e1SH?p=preview

[kimx]

Hi @Narretz
This error will not happen in every time.My test way is use developer tool disabled cache and press F5
refresh the page.If it still can't reproduce this error maybe modify .js file typing some withe space or press F5 more times.
please try it again thank you.
http://plnkr.co/edit/eIoapMVC9LQAAO2RCPTj?p=preview

[Narretz]

Ok, now I can see it. Only in Chrome though. I suspect it's a timing problem caused by 652b83e - we should investigte in that direction.
As a workaround you can try to give your directive multiElement: true. This worked in my case.
However, your example directive won't work correctly. ngRepeat transcluded the element, and therefore the animation disabling will not be moved to the single repeat elements. You should disable animations on the parent element instead.

[kimx]

Thanks for your suggestion.I will follow it to modify my directive.

[Narretz]

@dcherman maybe you want to take a look at this. It seems to be a regression caused by the lazy transclusion, although it strangely doesn't happen all the time

[dcherman]

Sure. Pretty surprising that this only happens some of the time for sure. The gist of the problem is that before lazy compilation, the nextSibling relationships of the template element was still intact at the time of compilation.

The absolutely nuts thing here that I haven't worked my way through is that sometimes, the elements lose their nextSibling relationships and parentNode. This is when you observe the error being thrown. Other times, the nodes become a child of a document fragment which maintains the sibling relationships. This is when it works.

In conclusion...wat? Thinking about what we can do to maintain those sibling relationships in the replaceWith step; that should fix the problem here.

[dcherman]

Even more nuts - the code always did the replaceWith step before compiling the $template elements for future use, so you'd imagine that we'd see the same non-determinism in older versions, but nope.

[dcherman]

@Narretz So....I'm not sure how to test case this yet, but I think there might be a bug in Chrome. That document fragment that's created in replaceWith seems to be getting disposed/garbage collected even though it should still have been referenced via parentNode of the template elements.

If I simply add a console.log(fragment) before returning from replaceWith, I completely eliminate the error since my console is still holding a reference to the fragment, but this should be happening by virtue of the parentNode property anyway.

The bug still exists if I replace the document fragment with document.createElement('div'), so it's not fragment specific.

[dcherman]

I think this might actually be fixed in Canary. I can't reproduce this there. If we need to push a fix prior to that, we just need to store a reference to the document fragment somewhere in order to avoid it being inappropriately collected.

If you think it's worth doing that, I'll look into it, otherwise this seems like it's just going to be naturally fixed with the next Chrome release.

[Narretz]

@dcherman Thanks for investigating! I think we can wait and see if it is fixed in the next Chrome release, since it should be possible to work around this issue by giving the custom directive multiElement: true, too. At least that seemed to work for me.

[kimx]

Hi @Narretz
I found another similar error that will happen first render when load from backend.
This error will not happen in every time.My test way is use developer tool disabled cache and press F5
refresh the page.If it still can't reproduce this error maybe modify .js file typing some withe space or press F5 more times.
please try it again thank you.
http://plnkr.co/edit/O5aQ48eHidvsYxO4Zip7?p=preview

Ps:Only Chrome will happen this error.

[aderowbotham]

FWIW, I'm seeing the same intermittent error with an ng-repeat-start / ng-repeat-end pattern nested inside a parent ng-repeat-start / ng-repeat-end on table rows as in the plunkr. Also only in Chrome - version 48.0.2564.116 (64-bit)

In my case there is no custom directive in use, so I can't add multiElement: true to anything.

The problem doesn't occur in Chrome 51.0.2673.0 canary (64-bit) (also it renders much faster!)

[kimx]

I've updated the latest Version 49.0.2623.87 m (64-bit) but It still happened.

[neoGeneva]

Does anyone have a work around for this until Chrome fixes it?

The only thing I could come with (that doesn't involve hacking the angular.js source) is overriding document.createDocumentFragment() but it seems less than ideal:

var createDocumentFragment = document.createDocumentFragment;
document.createDocumentFragment = function () {
    var fragment = createDocumentFragment.call(document),
        appendChild = fragment.appendChild;

    fragment.appendChild = function (newChild) {
        newChild.$$parentNode = fragment;
        return appendChild.call(fragment, newChild);
    };

    return fragment;
};

[bronielsen]

I am seeing the same problem with nested ng-repeat-start/end's. In my case it is repeatable in my (relatively large) application. But I have been unable to figure out how replicate it in a plunker environment.

I can change the controller function that sets up the data for the inner ng-repeat-start/end loop in a way that makes the problem goes away. Unfortunately, the functionality that I need is also gone :-/ so it is not a solution for me.

I also figured out that the problem is that the next-sibling relationship is gone (or not yet established), so I get a "Unterminated attribute, found 'ng-repeat-start' but no matching 'ng-repeat-end' found." My guess is also that this is related to lazy compilation problem.

I am running Angular 1.5.0 and Chrome 49.0.2623.87 m

This is a serious problem for me and I have been stuck for two weeks trying to work my way around it, without having to refactor this part of my application significantly. I have a requirement to be able to run on Chrome.

--Morten

[dcherman]

@Narretz Seems like this issue might be getting enough traction to warrant a patch. On the 6 week release cycle, the next Chrome release is on April 19th, so the natural fix for this is just under a month out.

Happy to make a pull request for this if you'd like to do a patch release prior to Chrome 50 being out (just re-confirmed that it still seems to contain the GC fix). Should be extremely simple to add a temporary workaround.

[Narretz]

@dcherman sounds like a good idea. Then we just have to release 1.5.3 before the Chrome release. ;) Can you please add a TODO to the fix to remind us that we should removed this after Chrome 50 is released?

[bronielsen]

@Narretz, @dcherman. Hi Guys. I would really appreciate a work around for this. I am so very stuck. My only alternative seems to be an unproductive refactoring of the code, which will be wasted when the fixed Chrome is released and I will go back to the code as it is now. So lots of thanks, if this is possible!

[dcherman]

@bronielsen There's not a great one. As a play on the one provided by @neoGeneva , it seems like simply adding the document fragment to a WeakSet helps to work around the problem, but it avoids the problem where the document fragment may truly have been GC eligible but was prevented from being collected due to the $$parentNode reference. This isn't a problem in the PR referenced since we know that the lifecycle of the template elements and parent document fragment should be identical, so a hard reference is fine there.

(function() {
  var chromeUserAgent = /Chrome\/(\d{2})/.exec(navigator.userAgent);

  if (chromeUserAgent && Number(chromeUserAgent[1]) < 50) {
    var set = new WeakSet();
    var createDocumentFragment = document.createDocumentFragment.bind(document);

    document.createDocumentFragment = function () {
        var fragment = createDocumentFragment();
        var appendChild = fragment.appendChild;

        fragment.appendChild = function(newChild) {       
          set.add(this);    
          this.appendChild = appendChild;
          return this.appendChild(newChild);
        };

        return fragment;
    };
  }
}());

Definitely some ugly looking workaround, so I wouldn't leave that in your code long-term.

[bronielsen]

@dcherman I manually implemented the change in pull request #14286, and it made the problem go away for the current debug-version of my code. Fantastic! I was just completely stuck here. Thank you.