ngRepeat breaks when re-created by ngIf under certain circumstances

[tlaytongoogle]

Note: for support questions, please use one of these channels: https://github.com/angular/angular.js/blob/master/CONTRIBUTING.md#question. This repository's issues are reserved for feature requests and bug reports.

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

Bug

What is the current behavior?
When an ngRepeat is with an ngIf which is set to true, then false, then true again (re-creating the
ngRepeat), if the HTML is scructured in a particular way, if causes the ngRepeat to throw and error and fail to render.

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 bug can reproduced using https://jsfiddle.net/ndhrpz06/23/

Note some salient changes to that repro code which (each individually) cause the error not to occur

• if the 'ng-if' attribute of the <div> around the test-dir and ng-repeat is removed
• if the ng-repeat is removed or the repeated array remains empty
• if the extra <div> wrapping the ng-repeat is removed
• if the 'replace' setting testDir is set to false instead of true,
• if the 'ng-if' attribute of the testDir template's root <div> is removed, or that <div> is wrapped in something else so that it isn't the root element
• if the testDir is replaced in the main HTML with its own template string
• if the testDir template string is applied directly to the 'template' setting of the dir, rather than via 'templateUrl' and $templateCache

What is the expected behavior?

The ngRepeat element should render normally without any errors.

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

The current behavior is broken. In particular, it can cause unexpected breakages when upgrading from 1.4 to 1.5.

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.

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

The error message which is thrown:

angular.js:13550 TypeError: Cannot read property 'parent' of undefined
at Object.enter (angular.js:5396)
at ngRepeatTransclude (angular.js:29009)
at publicLinkFn (angular.js:8276)
at lazyCompilation (angular.js:8615)
at boundTranscludeFn (angular.js:8414)
at controllersBoundTransclude (angular.js:9143)
at ngRepeatAction (angular.js:29003)
at $watchCollectionAction (angular.js:16938)
at Scope.$digest (angular.js:17073)
at Scope.$apply (angular.js:17337)

This may seem like a pretty esoteric bug, but it seems to be indicative of a deeper problem with the directive rendering logic. I would't be surprised if there are other edge condition issues caused by the same underlying bug.

[Narretz]

You briefly said there's a difference betwenn .4 and 1.5? Could you please what the last working version is?

[tlaytongoogle]

It works in 1.4.10 (https://jsfiddle.net/7pho1pys/3/), which I believe is the last 1.4 version.

It breaks in 1.5.0 (https://jsfiddle.net/7pho1pys/4/), although only once the ngRepeat array has at least 2 elements in it rather than just 1, and the error message is different:
angular.js:13236 TypeError: Cannot read property 'nextSibling' of undefined
at ngRepeatAction (angular.js:28560)
at $watchCollectionAction (angular.js:16529)
at Scope.$digest (angular.js:16664)
at Scope.$apply (angular.js:16928)
at HTMLButtonElement. (angular.js:24551)
at defaultHandlerWrapper (angular.js:3398)
at HTMLButtonElement.eventHandler (angular.js:3386)

It breaks in 1.5.1 (https://jsfiddle.net/v6fmwcy6/3/) with the current error ("Cannot read property 'parent' of undefined")

[Narretz]

Looks like this is another victim of lazy compilation of the transclude function (used by ngIf), introduced in 1.5.0-beta.1. Your example works in 1.5.0-beta.0.

It appears that the test-dir with the replace, templateUrl + ngIf combination AND the additional div wrapper around the ngRepeat: <div><div ng-repeat="v in mainCtrl.values">{{v}}</div></div> causes the ngRepeat to have a wrong reference when it needs to add the repeated element into the DOM. When the repeated node gets added, previousNode is undefined (while it should be the placeholder comment generated by ngRepeat):

angular.js/src/ng/directive/ngRepeat.js

Line 536 in 9355aea

$animate.enter(clone, null, previousNode);

When you either remove the ngIf from the template or use a local template or remove replace, then it works. Still something that shouldn't happen ideally. You can also remove the wrapper div, but I think it only works somehow, and still not like it should.

However, note that replace is deprecated, and you shouldn't use it ideally. I'm not sure if this can be fixed.

@dcherman do you want to take a look at this?

[dcherman]

@Narretz I'm game. Might be at least until this weekend before I have time to really dig in since this one might be tricky. The gist of the cause looks to be that the $element passed to the ngRepeat's link function is of .length 0, so it doesn't even have the comment node there. Tracing that back, it's linking the ngRepeat against the (non-existent) children of a text node.

Basically, it looks like the NodeList during compilation is getting its offsets messed up somewhere here.

[Narretz]

Yeah, the resulting DOM looks broken. In 1.5.5, the starting comment for the ngIf at the root of the templateUrl is missing:

      <!-- ngIf: mainCtrl.on --><div class="ng-scope" ng-if="mainCtrl.on">

        <div class="ng-scope" test-dir="" ng-if="true">Hi from remote</div>
        <!-- end ngIf: true --
        ><div><!-- ngRepeat: v in mainCtrl.values --></div>
        <div>The ng-repeat currently exists</div>
      </div><!-- end ngIf: mainCtrl.on -->
      <!-- ngIf: !mainCtrl.on -->

It should look like this:

      <!-- ngIf: mainCtrl.on --><div class="ng-scope" ng-if="mainCtrl.on">
        <!-- ngIf: true -->
        <div class="ng-scope" test-dir="" ng-if="true">Hi from remote</div>
        <!-- end ngIf: true -->
        <div><!-- ngRepeat: v in mainCtrl.values --><div class="ng-binding ng-scope" ng-repeat="v in mainCtrl.values">0</div><!-- end ngRepeat: v in mainCtrl.values --></div>
        <div>The ng-repeat currently exists</div>
      </div><!-- end ngIf: mainCtrl.on -->

[lgalfaso]

In fact, I think this is another variation of replace: true, templateUrl: $some_template_with_an_ng_if_at_the_top_element.

The reason being the following:
when we first run the linking phase of the content of

<div ng-if="mainCtrl.on">
    <div test-dir></div>
    <div><div ng-repeat="v in mainCtrl.values"></div></div>
    <div>The ng-repeat currently exists</div>
  </div>

we when calling compositeLinkFn then nodeList is the following:
0. text node for the spaces

1. <div test-dir></div> // This is here as this is a templateUrl
2. text node for more spaces
3. <div><div ng-repeat="v in mainCtrl.values"></div></div>
4. text node for even more spaces
   [other stuff that we do not care]

Later, the directive testDir completes its compilation and because it has an ngIf at the top element, then <div test-dir></div> is removed not replaced with a comment in nodeList (remember that nodeList is live)

when you toggle twice, the transclusion kicks in again this time nodeList has the following element
0. text node for the spaces

1. text node for more spaces
2. <div><div ng-repeat="v in mainCtrl.values"></div></div>
3. text node for even more spaces

but in linkFns we mark that we expect the ngRepeat to be at position 3 (at it would be expected if <div test-dir></div> was replaced with a comment node.) Then we call compositeLinkFn on the child nodes of the node at position 3 (that now is a text node), this is a list of 0 elements and continue the recursion as if it this was the children of <div><div ng-repeat="v in mainCtrl.values"></div></div>. From this point on, things are already broken. Adding an element to be displayed in the ngRepeat just makes the error simple to see

[Narretz]

Thanks for investigating @lgalfaso ! I wonder why this is expected:

Later, the directive testDir completes its compilation and because it has an ngIf at the top element, then <div test-dir></div> is removed not replaced with a comment in nodeList (remember that nodeList is live)

What's the exact reason the element is removed and not replaced? Why can't we replace it?

[YuliaTsareva]

Any news on this issue, guys?
I believe we've encountered with this bug after migration from Angular 1.4.8 to Angular 1.5.7.

There is a directive with replace: true and ng-repeat in its template. And next to it there is another directive which calls element.html() in its link function. Both of them are placed in a container with ng-if on it. When they appear for the first time, everything works well. But for the second time element passed to link is incorrect.
I've done a research and found that the reason is different DOM structure (absence of an opening comment ''<!-- ngRepeat:").

I created a simple demo: https://plnkr.co/edit/mJxbEhXoV9PLvEVx44rf?p=preview
Notice that when you click on the button for the first time it writes to console

element passed to otherDir [other-dir]

and for the second time

element passed to otherDir [text]

[Orvisky]

I have same problem.

Simplified, we iterate throught multi-dimensional data array of products and items, where product and item has own directive. We have we downgrade from 1.5.7 do 1.4.12

$scope.dataArray = [
{
name: 'Product 1',
items: [A, B, C, D]
},
{
name: 'Product 2',
items: [A, B, C, D]
}
];

• Expected visual

Product 1
A
B
C
D

Product 2
A
B
C
D

• Bugged visual

Product 1

Product 2
A
B
C
D
A
B
C
D

[petebacondarwin]

I will look into this but in the meantime please consider removing the replace: true from your directives. It is deprecated primarily because it creates very difficult bugs and in almost all situations it can be worked around.