This repository has been archived by the owner on Apr 12, 2024. It is now read-only.
Weird interaction between nested forms and directives #3935
Milestone
Comments
|
might have something in common with #3923 |
|
might also be fixed in RC3 with this PR (#3927) |
|
@nilpath - Can you check against master since there has been some work on this area of the code? |
|
@petebacondarwin tested with master (v1.2.0-e4415d2) (plunker: http://plnkr.co/edit/uY6W1n6NMBROUG66V7Fh?p=preview) and it seem to still have the same problem |
|
The problem is in timing. The inner form controller is created (and the directive is linked) after the ngModel directive of the inner form element. That leads to the element being registered (via $addControl) on the outer form as the inner form controller is not associated with the |
|
The PR #3927 should fix this I think. |
|
Here's the plunkr with the patch from PR #3927: http://plnkr.co/edit/Hr0th7kr3eVGves5GFd4?p=preview Seems like Vojta's PR fixes it, if I understand the test correctly. |
|
nicley done! 👍 |
jamesdaily
pushed a commit
to jamesdaily/angular.js
that referenced
this issue
Jan 27, 2014
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
jamesdaily
pushed a commit
to jamesdaily/angular.js
that referenced
this issue
Jan 27, 2014
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
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Hi
I've just updated my code to use angular 1.2-rc2 and found that when using a custom directive with replace: true and that includes an ng-form, the form elements are attached to the parent form rather than the nested form.
changing the templateUrl to an inline template or setting replace: false solves this but doesn't seem right that it's not working with this particular setup as well..
heres a plunkr with 1.2-rc2
http://plnkr.co/edit/zcvaSl13HgkSiuCOiMut?p=preview
The text was updated successfully, but these errors were encountered: