ngAnimate insertBefore error

[brianconnoly]

First I see this error

Then I go there

And do this

Problem solved till next angular.js update.
What am I doing? )

[petebacondarwin]

@brianconnoly - Can you provide the application code that triggers this error?

[brianconnoly]

It appears in several cases of using ng-repeat in manually compiled elements.
The app is quite big now.

[kiddkai]

using with ui-sortable , but it seems that ui-sortable works correctly, dono why have this issue

[ajshapiro]

I've experienced this as well since Angular 1.2.0-rc.3 when using ui-multi-sortable and jquery ui sortable. Not only is the insertBefore error generated, but the nodes are moved around improperly. I believe this is the same issue as #4786 and is the result of the addition of the addition of comments in 1.2.0-rc.3, which may be causing the default $AnimateProvider enter method to corrupt the ngRepeat output.

Here's a JSBin that replicates the problem. If you change the angular.js src to 1.2.0-rc.2 instead of 1.2.1, it works fine. Also removing the line in angular.js that adds the "end ngRepeat" comment also seemed to resolve the problem, although in the issue linked above, it is suggested that ngRepeat relies on those end comments, so not sure if this may cause other problems.

http://jsbin.com/asOXAXOz/1/edit?html,js,output

[CGenie]

@matsko: You can just as well move if(parentNode) outside of forEach, for performance reasons :)
Anyways, this seems to be ng-repeat-related. You're changing some function of ng-animate which I don't think is the full source of problems here.
Anyways, it would be best if you commited your change to some forked branch of yours and see if all Travis tests pass.

[matsko]

I will be taking a solid look at this once this once this PR gets in #5015. Thank you everyone for the detail and research put into it.

[matsko]

@brianconnoly sorry, but I had to change the milestone to 1.2.3 since there are a few other issues very similar to this one and I want to get them all fixed with the same patch. Version 1.2.3 shouldn't be longer than a week or two away from tomorrow.

[egrajeda]

Since this thread has more discussion than #4786, I just wanted to add that commit 9efa46a seems to be the cause of the problem.

[kamilkp]

I have the same problem, and a sample app where this error occurs: Issue #5054

[matsko]

@brianconnoly Just fixed a bunch of the comment-node issues with ngAnimate/animate.js. But this code looks like it's inside of ng/animate.js instead of ngAnimate/animate.js. Which means it's a core issue. The fix that you have in your screenshot is a great fix, I just need an example so I can craft a good test to isolate the bug.

[matsko]

@brianconnoly could you please answer my previous question?

[kamilkp]

Could you please say something in my issue too? #5054
You closed it already, but it doesn't seem closed to me.

[FeelBuzz]

@matsko my application is very complex... It's hard to locate certain occurrence. It's somewhere in digest. It appears several times in a row. And I think it could be somehow related with manually $compil'ed and appended blocks. With manually created scopes.

[matsko]

This may already be fixed with the fixes from 1.2.4, or it might have to do with multiple ng-directives being placed on the same element (ngRepeat + ngIf or ngSwitch + ngIf) that cause it. It's just a matter of finding which combination causes it.

[CGenie]

Nope, I just upgraded to angular 1.2.4 and the problem still persists.
My code is also complicated, but the main setting is that I use sortable to sort between different lists: http://jqueryui.com/sortable/#connect-lists
Sorting within one list does work properly, sorting between two lists behaves really strange: often many elements in the target list disappear.
Upon inspection it is quite obvious that angular's ng-repeat directive has problems getting the li's right: "ng-repeat end" comments end after first element, after that there's another

element but it doesn't have an "end" comment. If sortable moves only the

element, and leaves the comment behind, it breaks ng-repeat's logic of proper list rerender, in my opinion.

EDIT: scratch that: moving element to another ng-repeat cycle I break this cycle, different iteration variables, different scope, it's not the cause :)

[matsko]

Ya I've seen this before and moving elements visually from list to list is not the way to go.

[kamilkp]

What do you mean it's not the way to go? It worked with ngRepeat in previous versions of AngularJS (1.1.4 for instance). We cannot have sortable elements on our apps these days? What kind of enhancement is that? It's a regression to me. See #5054 for more insight.

[CGenie]

I agree, after trying to fix sortable for couple of hours I am still convinced it's the comments logic breaking things here.

[matsko]

The right way to do this is to handle the DOM clone + drag operation, but not to go through with the actual DOM insertion of the element into the new list or new position within the old list. Instead, on the onBeforeDragEnd event or so, just figure out what index the dragged element is and then do the add and removing or reordering on your own and let ngRepeat render again.

[kamilkp]

Yeah this will work, but it involves meddling inside the ui sortable libraries, Why did it work before without any of that and it doesn't work now with newer version of AngularJS.

[ajshapiro]

The problem with handling the drag but not doing the DOM insertion is that with the jqueryui sortable, once you start dragging, it is moving DOM elements (without moving the comment nodes) so even if you cancel and it reverts, it puts elements back in the wrong place relative to the comment nodes. The only workaround is to modify the sortable code to take a snapshot of the entire contents, including comment nodes, of each sortable and recreate when done. Else include the comment node with the dragged element. Neither is an ideal solution, IMO.

[matsko]

It worked before because ngRepeat didn't generate a series of inline comments to keep track of its elements. So if you sorted things then it wouldn't complain, and so long as you reordered the scope list afterwards as well then the repeat list would look the same.

[kamilkp]

So i have to write my own sortable logic (which within jQuery UI is about 250 lines of code).

It worked before because ngRepeat didn't generate a series of inline comments to keep track of its elements.

Why does it do it now?

The only workaround is to modify the sortable code to take a snapshot of the entire contents, including comment nodes, of each sortable and recreate when done. Else include the comment node with the dragged element.

Both non-trivial and library (jquery ui) invasive.

[ajshapiro]

Interestingly, for me, everything still works if I remove the line in angular.js that adds the "end ngRepeat" comment... have not noticed any side effects (though I am not clear what role those comment nodes play).

[CGenie]

Removing the 'end comment' element causes Travis tests not to pass. If I remember correctly, the ng-repeat-start/end logic broke, among others.

[kamilkp]

Yep, I've anticipated that this excessive comment nodes where added in order to make ng-repeat-start/end work (I wrote about it in #5054). Maybe we all should vote on that here. What functionality is more important to a developer: ng-repeat-start/end or ui-sortable? For me the latter.

[CGenie]

For me too, though I wouldn't count on AngularJS developers removing the ng-repeat-start functionality just because 2-3 of us voted so :)
I'd rather suggest removing the comments and using DOM data attributes as markers for loop start/end, for example.

[kamilkp]

I'd rather suggest removing the comments and using DOM data attributes as markers for loop start/end, for example.

I agree. In the meantime you can always do what i did in one of my apps. Download angularJS 1.1.4, copy ngRepeatDirective function form it and put it into angularJS 1.2.x javascript file. You will also need to copy the $animatorProvider and $animationProvider from 1.1.4, otherwise the DI will break. The downside of doing that is that the ngAnimate animations on ng-repeat won't work. But your app won't break, and you will be able to use ui-sortable and multi-sortable without concern.

[ajshapiro]

If the end comments are only necessary for ng-repeat-start/end to work, and given that it doesn't really make sense for sortable to work with start/end anyway (it expects a single element), then maybe a reasonable solution is to only insert end comments when ng-repeat-start/end are used (ie, clone is more than 1 element).

[matsko]

Going to close this for now. The ng-repeat drag and drop issues need to go into another issue. The source of the problem is too vague and I need to have a solid plunkr example to create a test so that I can close the insertBefore error. @brianconnoly please reopen as soon as you have a plunkr example. You can also email me directly so that we can have a video chat to find the bug in your application.

[chrisirhc]

@matsko , here's a Plunker that reproduces the ngRepeat issue.

http://plnkr.co/edit/ncQSgi4Y44P3ZyYM04Wv?p=preview

Instead of calling insertBefore, I called .css and ngAnimate is calling this on the comment element.

[kamilkp]

Check out my PR in this matter: #6016

[kamilkp]

I have written a module to obtain a sortable behaviour with no jQuery nor jQueryUI needed whatsoever. And it also doesn't break the ngRepeat inner logic. The module can be found here and a demo here