bug(ngTransclude): breaking change in 1.2.0-rc.2 #3923
Comments
|
@bekos - can you try out @vojtajina's PR, #3927, as this may well help out here? |
|
@petebacondarwin 👍 This PR works for my case :-) (Updated the plunker) |
|
I am going to close this one then. Thanks |
|
@petebacondarwin are you sure that #3927 is going to be merged? @IgorMinar wasn't sounding too reassuring... In any case this introduces a breaking change on the angular-ui/bootstrap. |
|
Something that fixes your issue will be merged. It may not be exactly On 11 September 2013 13:20, Pawel Kozlowski notifications@github.comwrote:
|
|
@pkozlowski-opensource - what is the issue in bootstrap? |
|
@petebacondarwin If I may help here, the flow that you see in the plunker is the same used in the The The problem now (rc2) is that the |
|
@petebacondarwin I would prefer that this one stays open till the fix is in master. I'm not sure if / when @vojtajina PR #3927 is going to be merged and this issue has nice reproduce scenario. I will monitor this issue and will close it as soon as a fix lands in master. |
How did compiling a templateUrl (async) directive with `replace:true` work before this commit? 1/ apply all directives with higher priority than the templateUrl directive 2/ partially apply the templateUrl directive (create `beforeTemplateNodeLinkFn`) 3/ fetch the template 4/ apply second part of the templateUrl directive on the fetched template (`afterTemplateNodeLinkFn`) That is, the templateUrl directive is basically split into two parts (two `nodeLinkFn` functions), which has to be both applied. Normally we compose linking functions (`nodeLinkFn`) using continuation - calling the linking function of a parent element, passing the linking function of the child elements as an argument. The parent linking function then does: 1/ execute its pre-link functions 2/ call the child elements linking function (traverse) 3/ execute its post-link functions Now, we have two linking functions for the same DOM element level (because the templateUrl directive has been split). There has been multiple issues because of the order of these two linking functions (creating controller before setting up scope locals, running linking functions before instantiating controller, etc.). It is easy to fix one use case, but it breaks some other use case. It is hard to decide what is the "correct" order of these two linking functions as they are essentially on the same level. Running them side-by-side screws up pre/post linking functions for the high priority directives (those executed before the templateUrl directive). It runs post-linking functions before traversing: ```js beforeTemplateNodeLinkFn(null); // do not travers afterTemplateNodeLinkFn(afterTemplateChildLinkFn); ``` Composing them (in any order) screws up the order of post-linking functions. We could fix this by having post-linking functions to execute in reverse order (from the lowest priority to the highest) which might actually make a sense. **My solution is to remove this splitting.** This commit removes the `beforeTemplateNodeLinkFn`. The first run (before we have the template) only schedules fetching the template. The rest (creating scope locals, instantiating a controller, linking functions, etc) is done when processing the directive again (in the context of the already fetched template; this is the cloned `derivedSyncDirective`). We still need to pass-through the linking functions of the higher priority directives (those executed before the templateUrl directive), that's why I added `preLinkFns` and `postLinkFns` arguments to `applyDirectivesToNode`. This also changes the "$compile transclude should make the result of a transclusion available to the parent directive in post- linking phase (templateUrl)" unit test. It was testing that a parent directive can see the content of transclusion in its pre-link function. That is IMHO wrong (as the `ngTransclude` directive inserts the translusion in its linking function). This test was only passing because of c173ca4, which changed the behavior of the compiler to traverse before executing the parent linking function. That was wrong and also caused the angular#3792 issue, which this change fixes. Closes angular#3792 Closes angular#3923 Closes angular#3935 Closes angular#3927
How did compiling a templateUrl (async) directive with `replace:true` work before this commit? 1/ apply all directives with higher priority than the templateUrl directive 2/ partially apply the templateUrl directive (create `beforeTemplateNodeLinkFn`) 3/ fetch the template 4/ apply second part of the templateUrl directive on the fetched template (`afterTemplateNodeLinkFn`) That is, the templateUrl directive is basically split into two parts (two `nodeLinkFn` functions), which has to be both applied. Normally we compose linking functions (`nodeLinkFn`) using continuation - calling the linking function of a parent element, passing the linking function of the child elements as an argument. The parent linking function then does: 1/ execute its pre-link functions 2/ call the child elements linking function (traverse) 3/ execute its post-link functions Now, we have two linking functions for the same DOM element level (because the templateUrl directive has been split). There has been multiple issues because of the order of these two linking functions (creating controller before setting up scope locals, running linking functions before instantiating controller, etc.). It is easy to fix one use case, but it breaks some other use case. It is hard to decide what is the "correct" order of these two linking functions as they are essentially on the same level. Running them side-by-side screws up pre/post linking functions for the high priority directives (those executed before the templateUrl directive). It runs post-linking functions before traversing: ```js beforeTemplateNodeLinkFn(null); // do not travers afterTemplateNodeLinkFn(afterTemplateChildLinkFn); ``` Composing them (in any order) screws up the order of post-linking functions. We could fix this by having post-linking functions to execute in reverse order (from the lowest priority to the highest) which might actually make a sense. **My solution is to remove this splitting.** This commit removes the `beforeTemplateNodeLinkFn`. The first run (before we have the template) only schedules fetching the template. The rest (creating scope locals, instantiating a controller, linking functions, etc) is done when processing the directive again (in the context of the already fetched template; this is the cloned `derivedSyncDirective`). We still need to pass-through the linking functions of the higher priority directives (those executed before the templateUrl directive), that's why I added `preLinkFns` and `postLinkFns` arguments to `applyDirectivesToNode`. This also changes the "$compile transclude should make the result of a transclusion available to the parent directive in post- linking phase (templateUrl)" unit test. It was testing that a parent directive can see the content of transclusion in its pre-link function. That is IMHO wrong (as the `ngTransclude` directive inserts the translusion in its linking function). This test was only passing because of c173ca4, which changed the behavior of the compiler to traverse before executing the parent linking function. That was wrong and also caused the angular#3792 issue, which this change fixes. Closes angular#3792 Closes angular#3923 Closes angular#3935 Closes angular#3927
This commit (bf79bd4) seems to break things for async directives, that use transclude.
Here is a plunker: http://plnkr.co/edit/pp4zKjoBF6oY6DfoSkv2?p=preview
The problem is that inner directive cannot find the
ngModelControlleron the outer directive, that usesng-transclude.This works fine with 1.2.0rc1 or if you replace the templateUrl with an inline template.
The text was updated successfully, but these errors were encountered: