$animateCss used on a directive that uses a templateUrl breaks

[CodySchaaf]

$animateCss ends up operating on an element that does not end up in the view after the directive is rendered.

As you can see in this plunker if you uncomment the template (and comment the templateUrl) the element is correctly styled and appears in the view. However when the directive is using tamplateUrl the element is not styled.

https://plnkr.co/edit/yO1AvvupoveRi3IWevVL?p=preview

This was not the case in 1.4.x, but is a new issue when upgrading to 1.5.

While the directive is using templateUrl you can also check the console and click reveal in elements pane to see that no element is revealed.

This issue means that you can not do any clean up in the done function or any further styling (my issue is I want to scroll the element into view once it is expanded since the element would be too short to scroll into view before. You can also see that '-active' and '-add-active' styles are not removed.

[CodySchaaf]

I also see this error in my own project Cannot read property '$$ngAnimateParentKey' of null which comes from this line

return (c.$$ngAnimateParentKey || (c.$$ngAnimateParentKey = ++K)) + "-" + a.getAttribute("class") + "-" + b

[Narretz]

Thanks for reporting. Can you please create a plnkr that repdroduces the error you mentioned?

[CodySchaaf]

There is a plnkr included in the original story, here it is again. Let me know if this one doesn't work.

https://plnkr.co/edit/yO1AvvupoveRi3IWevVL?p=preview

[Narretz]

@CodySchaaf You said you have an error in your project. I was specifically asking for a repro for that error, because your posted plnkr doesn't throw an error.

[Narretz]

Also, as far as I can see, the behavior in 1.4.9 and 1.5.0 is the same: no animation with templateUrl.

[Narretz]

So I doubt this worked in 1.4.x, because this inherently happens because with templateUrl, it takes at least a few milliseconds until the template is available and the direcitve is linked. But the animation runs before that happens, so there's nothing to animate. This is a known limitation. You can fix this by putting the template in the cache: https://plnkr.co/edit/o7SyQNY5oMZxAKVh7Eeg?p=preview

[CodySchaaf]

Oh that makes sense. Sorry.

I will try to get a working repro for that. I suspect it has to do with angular attempting to preform some clean up action on the element that expects it to have a parentNode since c is var c = a.parentNode and a is the element in question (sorry about using the minified code).

Looks like you are correct with 1.4.9 having similar behavior in the plnkr, the issue in my application was not present in 1.4.9 though so I will dig deeper.

Thanks for your help.

[CodySchaaf]

Sounds like there might be a different issue then, thanks for your help. It definitely worked in app before the upgrade, so maybe it is just timing changes that have surfaced this issue now with 1.5. I'll report back if I can get a plnkr that is broken in 1.4.9 but working in 1.5.0.

Probably wouldn't be an issue in production since the templates are cached there, but we don't currently cache them in development (might be a good time to get the environments in sync).

[Narretz]

1.5.0 includes lazy compilation, which can cause subtle issues. It's only active when the directive uses transclusion, though (and should not cause differences between template / templateUrl)..

[CodySchaaf]

@Narretz I was able to make a plnkr that works on 1.4.9 but is broken on 1.5.0

1.5.0 https://plnkr.co/edit/uP3HxlLpn9Gyzs3mFKLT?p=preview 1.4.9 https://plnkr.co/edit/RcOKBj9xKX6DYIFKVR9E?p=preview

Just click ready and you'll see it change color and scroll (if screen is less than 1000px tall) on 1.4.9 but not on 1.5.0

You'll also see the template is loaded before you hit ready on 1.4.9 but not until after 1.5.0 (view the network requests in the console).

This is probably because of the lazy compile, although there is no transclusion taking place, so I'm not sure why.

[CodySchaaf]

Also here is an example with the error, you need to have debugging enabled through the $logProvider, as well as an animation stagger.

https://plnkr.co/edit/s4H67PyJ69Z4ZA2DHPao?p=preview

[Narretz]

For the first problem - ngIfuses transclusion, so that is what is causing this. I'll investigate if we can work around this. edit: it looks like the animation starts, but doesn't finish, so there seems to be a real bug here. For the second problem, I have a hard time reproducing the error. I've seen it once in Chrome, but I actually had to disable the debug logging. Does it happen in all browsers for you?

[Narretz]

@dcherman Here's another issue I'd like your opinion on. https://plnkr.co/edit/uP3HxlLpn9Gyzs3mFKLT?p=preview

In this case the lazy compilation introduces a timing problem that breaks enter animations. There's an element with ngIf that has another directive that has a templateUrl. I think what happens is that since the compilation is delayed, the animation starts on an element that gets then "replaced" by the result of the templateUrl directive at this line: https://github.com/angular/angular.js/blob/master/src/ng/compile.js#L2701 (when you log firstElementToRemove it'll have animation related data on it) Because the animation is already running, the whole animation process breaks (because it relies on inline styles, element data, and event listeners). In this case it doesn't even look like caching the template helps - you can only work around this by using template instead of templateUrl. So far the only solution I can think of is to use eager compilation if there's a templateUrl directive on the element that uses a transclusion.

[dcherman]

@Narretz That timing bug always existed actually. If you invert the condition and have ctrl.ready be true at compile time, the ngIf always transcluded before the templateRequest finished returning which would result in the beforeTemplateLinkNode !== beforeTemplateCompileNode condition being true, so it exhibits the same bug.

I'm still thinking if there's anything that we can do in order to avoid this in the general case since causing eager compilation won't actually fix the bug here.

[CodySchaaf]

I was able to repro pretty consistently yesterday in chrome, but I might have been doing something weird with the debugger. It seems to be less consistent today though. If you bump up the stagger index, and stagger, you can get a more consistent repro though, although not perfect.

https://plnkr.co/edit/s4H67PyJ69Z4ZA2DHPao?p=preview

[Narretz]

@CodySchaaf Ok, now the error pops up more often. I can't reproduce it in Canary or FF though, so it might be a Chrome bug that is going to be fixed in the future.

@dcherman Oh, yeah that makes sense. I guess this limitation now causes issues more often because of lazy compilation. Would be cool to fix this directly in the compiler, although I can't say I see how.

[dcherman]

@Narretz I've got some stuff to ship this week so I doubt I'll really be able to do much looking into this (unless I somehow find free time after work) until at least next week.

My initial curiosity on this is that we're replacing an element that has already clearly been cloned via transclusion at this point, and does that really need to happen? It seems like this element is inappropriately being replaced, so that's where I was going to start looking, starting with just seeing which (if any) unit tests break with that code removed for this specific scenario before really considering how to refactor the condition for replacement and what side effects that might have.

[Narretz]

@dcherman No probelm. It's not high priority. I was wondering about the replacement issue too.Maybe we can optimize this.

[CodySchaaf]

Since this prevents me from being able to upgrade from 1.4.* do you think I should focus on finding a work around, or is there a chance it will we looked at in the near-ish future? I know it probably isn't a high priority, so just wanted to get a time frame.

Thanks for all you guys do!

[Narretz]

@CodySchaaf I'll probably won't be able to look into it for some time. It's also pretty complex, so I doubt there's an easy solution, unfortunately.

[michi88]

I've also been running into this problem when upgrading from 1.2.x (yes, I know) to 1.5.2. Just wanted to share this plnkr where you see this clearly happening.

https://plnkr.co/edit/okAirW34LQ8uCYzR7Ijf?p=preview

When you click 'Add message' the first one is not animated as it's not transcluded yet. All after that are fine.

In this screenshot you can clearly see that the element is not yet transcluded:

This issue is the root cause I guess: https://github.com/angular/angular.js/issues/13884

I think there should at least be something about this in the release notes if this is not going to be fixed. Maybe even a note in the animation docs although it's not exclusive to that.

[michi88]

Alright, after reading the docs here: https://docs.angularjs.org/api/ng/service/$compile#-templateurl- I do think this can be deemed expected behaviour. You guys could consider to try to wait for the compile to finish somewhere in the angular-animate code.

[gkalpak]

@michi88, your issue is the same as #15510 (and might be slightly different (or a subset) of that from the OP). This comment applies to you too.

tl;dr

15514 would fix your issue as long as your templates are cached by the time the directive starts compiling (e.g. by pre-caching them into $templateCache during your build process).

I just found this issue and I believe I have accidentally addressed the problem caused by replacing the node during lazy compilation in #15514 (as far as enter animations are concenrned). I have looked into the compiler bits and couldn't identify a quick improvement for forgoing the replacing in certain cases (it would be nice if it were possible - but #15514 would still be necessary for certain cases).

That said, I think there might be another (possibly similar) bug related to staggering (also mentioned in #14124). I'll try to look into it in the following days :smiley:

[gkalpak]

That said, I think there might be another (possibly similar) bug related to staggering

The issue is actually related to $animateCss (regardless of staggering) and is identical in nature with #15514. A minimal reproduction can be found here.

This one is quite complicated to solve (without animations falling apart) and I also had a hard time writing a failing unit test. If someone could write a failing unit test, I could give it a go.

The preconditions for the bug are:

1. Using $animateCss.
2. Animating a transcluded node.
3. The animated node having a templateUrl directive on it.
4. The template has not been already cached.

Due to (4), this issue only affects entering animations, since in all other cases the template would be cached (as part of having compiled the element).

The easiest workaround is to make sure templates are cached. This can be done:

1. By explicitly pre-populating the $templateCache (e.g. as a build step or at runtime).
2. By having previously compiled the directive (and thus fetched the template).

[BhanuSingamsetty]

User $timeout function to avoid "Uncaught TypeError: Cannot read property '$$ngAnimateParentKey' of null".