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

fix(compile): fix directives with controller, templateUrl and compile properties #3514

Closed
wants to merge 1 commit into from

Conversation

jankuca
Copy link
Contributor

@jankuca jankuca commented Aug 8, 2013

This PR addresses the following JSBin: http://jsbin.com/azomeh/9/edit

Directives combining templateUrl and a controller were not getting compiled correctly. The second call to applyDirectivesToNode was being made with the controller property of the directive set to null which made it impossible for the compiler to find the controller constructor function.

@mary-poppins
Copy link

Thanks for the PR!

  • Contributor signed CLA now or in the past
    • If you just signed, leave a comment here with your real name
  • PR's commit messages follow the commit message format

If you need to make changes to your pull request, you can update the commit with git commit --amend.
Then, update the pull request with git push -f.

Thanks again for your help!

controllerDirectives[directiveName], directive, $compileNode);
controllerDirectives[directiveName] = directive;
}
directiveValue = directive.controller;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

given your change, this line looks unnecessary

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

~500 tests break when I remove it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, I tried to remove it again and it works.

@IgorMinar IgorMinar closed this in 5c56011 Aug 9, 2013
@IgorMinar
Copy link
Contributor

Thanks Jan! I reviewed the code and this is the correct fix.

IgorMinar pushed a commit that referenced this pull request Aug 9, 2013
Controllers should be always instantiated after compile fn runs, but before
pre-link fn runs. This way, controllers are available to pre-link fns that
request them.

Previously this was broken for async directives (directives with templateUrl).

Closes #3493
Closes #3482
Closes #3514
IgorMinar pushed a commit to IgorMinar/angular.js that referenced this pull request Aug 10, 2013
This fixes regression introduced by angular#3514 (5c56011) - this commit is being
reverted here and a better fix is included.

The regression caused the controller to be instantiated before the isolate scope
was initialized.

Closes angular#3493
Closes angular#3482
Closes angular#3537
chirayuk added a commit that referenced this pull request Aug 12, 2013
This fixes regression introduced by #3514 (5c56011) - this commit is being
reverted here and a better fix is included.

The regression caused the controller to be instantiated before the isolate scope
was initialized.

Closes #3493
Closes #3482
Closes #3537
Closes #3540
chirayuk added a commit to chirayuk/angular.js that referenced this pull request Aug 21, 2013
This fixes regression introduced by angular#3514 (9c51d50) - this commit is being
reverted here and a better fix is included.

The regression caused the controller to be instantiated before the isolate scope
was initialized.

Closes angular#3493
Closes angular#3482
Closes angular#3537
Closes angular#3540
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.

3 participants