Skip to content
This repository has been archived by the owner on Apr 12, 2024. It is now read-only.

fix(compile): directive controller order fix #2738

Closed
wants to merge 1 commit into from

Conversation

lgalfaso
Copy link
Contributor

When there are two nested directives that the parent directive defines a controller, has transclude: true, has a templateUrl, the template has on the top level element the directive ng-transclude then the directive ng-transclude is applied before the parent directive. Now, if there is a nested child directive that also defines a controller and this controller needs to reference the parent controller, then given that ng-transclude was applied before the parent directive controller, the child directive is not able to reference the parent directive controller.

This patch changes this so the directives on the top level element of the template are applied after the directives on the original node

When a directive defines a templateUrl, directives on the top level
element will be applied after the directives on the original node
even if the directive defines `replace: true` and `transclude: true`

BREAKING CHANGE:
* The compilation and linking order of directives that are defined
on the top level element of an external template are compiled and
linked after the directives of the original node
@petebacondarwin
Copy link
Contributor

This looks like it could solve a number of issues that have been raised recently: #3238, #1805, #2533.

@IgorMinar
Copy link
Contributor

I cleaned up and simplified the test: 5b2223a46c22b64b3dc62b86eee472918ae7b3a2

@IgorMinar
Copy link
Contributor

this is a real bug but I'm not sure if the proposed solution is the right one. investigating...

@lgalfaso
Copy link
Contributor Author

@IgorMinar I am not sure if this is the proper solution or if something more elaborate is needed. One way or another, it is very important for the test to have require: '^parentDirective' that at 5b2223a46c22b64b3dc62b86eee472918ae7b3a2 is commented out.
Thanks for looking into this

@IgorMinar
Copy link
Contributor

I commented it out just to get over the error, it is now uncommented.

IgorMinar added a commit to IgorMinar/angular.js that referenced this pull request Jul 22, 2013
Previously it was possible to get into a situation where child controller
was being instantiated before parent which resulted in an error.

Closes angular#2738
@IgorMinar IgorMinar closed this in 45f9f62 Jul 22, 2013
IgorMinar added a commit that referenced this pull request Jul 22, 2013
Previously it was possible to get into a situation where child controller
was being instantiated before parent which resulted in an error.

Closes #2738
@paulyoung
Copy link

Are there any plans to create a new (possibly unstable) version containing this fix?

@aeby
Copy link
Contributor

aeby commented Aug 14, 2013

@paulyoung I guess it is in 1.2.0rc1

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants