angular / angular.js

AngularJS - HTML enhanced for web apps!
https://angularjs.org
MIT License
58.81k stars 27.5k forks source link

Directives got duplicated after an $animate.enter since beta.5 #8895

Closed Foxandxss closed 10 years ago

Foxandxss commented 10 years ago

I got an issue where a directive was being created twice when I did some $animation.enter.

The issue started on my repo: foxandxss/angular-toastr#18 so I started researching about it and the change was on beta.5 (when $animate.enter started to prepend instead of append).

I managed to reproduce it here: http://plnkr.co/edit/oYmXPIeN16g75a3kQJQc?p=preview

Starting from the console error (In my code I use enter like that and I get no error tho). Technically that shouldn't happen (I think).

Leaving that aside, you can see the directive twice on screen. If you change templateUrl to template it works (much like #6006).

So I was wondering if I did wrong on the plunker (adding a third parameter resolves the issue) or there is a bug.

btford commented 10 years ago

@matsko any ideas?

matsko commented 10 years ago

This looks like it's a bug with the compiler / interpolation code. The prepending causes it to somehow recompile itself and loose track of the node order. @tbosch can you have a look?

tbosch commented 10 years ago

yes, will do

jeffbcross commented 10 years ago

After reviewing this with @tbosch we believe that the example should not be supported, as changing DOM within controllers is strongly discouraged. If the animation logic in the example were moved to the directive's postLink function, where DOM manipulations are permitted, the issues go away.

See updated example: http://plnkr.co/edit/KpFedIafuuPamEhtl5f0?p=preview

tbosch commented 10 years ago

Here is an updated plunker that shows the effect even without ngAnimate: http://plnkr.co/edit/gfYEpxitqe4ohFzj7oFf?p=preview

The call to $animate.enter will essentially call body.prepend(container), which will then result in an error during the compile process (see dev tools). The fact that you see the template doubled is only a side effect of the error in the dev tools, which also occurs when you use an inline template (see the commented line in the plunker).

Yes @jeffbcross is correct in saying that you should not modify the DOM in the controller, especially as the controller constructor function is called in the prelink phase (see https://docs.angularjs.org/api/ng/service/$compile#pre-linking-function).

Closing this...

Foxandxss commented 10 years ago

@jeffbcross @tbosch The plunker is on a controller just for demo purposes. My real code is on a factory. It is a factory that generates directives (a factory to generate popup messages). There it happen too.

Well, I will workaround then.

tbosch commented 10 years ago

Yes, but same reasoning there. You can also try to replace $animate.enter with body.prepend, just like I did in the example, and you should see the same error.