fix($compile): allow compile data to be GCed after a non-clone linking #13422
Conversation
|
I believe this has the potential to break some people's apps, simply because it's not obvious how you are supposed to use $transclude. But it's good for 1.6.x, with a BC notice. |
|
Anything that breaks was probably already broken, although this might make it more broken (by throwing). Which makes me think this should also be done on link functions, not only on the transclude functions... |
jbedard
force-pushed
the
11618-trans-gc
branch
from
d44e5d3 to
45cc571
Compare
jbedard
force-pushed
the
11618-trans-gc
branch
from
45cc571 to
615f8a2
Compare
|
The transclusion slot stuff made this a bit more complicated so I've simplified it to only null out the link function data (not transclusion function data - which really just wraps the link function data). This is probably a better first step anyway... |
|
I think we can try it for 1.6. We should probably keep a list of possibly problematic commits for when we will snyc 1.6 to g3 in the future. |
|
I've added an error doc. |
|
1.6? 😁 |
jbedard
changed the title
|
Even though it's unlikely that this is going to break apps, I feel like we should mark this as "fix", just so that it shows up in the changelog, and when people have their app broken, they might see what's causing it. |
Previously the following would invoke the elemnet link function multiple
times, causing unknown and potentially buggy results:
var link = $compile(html);
link(scope);
link(scope);
This was always unsupported. Now this throws a multilink error.
jbedard
force-pushed
the
11618-trans-gc
branch
from
b350f03 to
2fe5773
Compare
jbedard
changed the title
|
I've updated the commit message |
|
Finally landed! Thanks for your preserverance jbedard. Due to how easy it is to make this mistake, I'll leave it in 1.6 only (for now). Especially since with some simple directives (such as interpolation), you won't notice any difference in behavior when linking multiple times. |
|
I think just 1.6 is best unless there's a strong reason to put it into 1.5. This can be considered a BC for those using |
This does two things:
transcludelink completes to allow mainly theboundTranscludeFnfunc/closure$compileNodes/compositeLinkFnto be GCedtranscludelink is executed again since that would either re-link the same element or clone the linked element