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

fix($compile): fix missing error handling when new-scope directive is applied before isolated-scope directive #4421

Conversation

buunguyen
Copy link
Contributor

Given dirA, which requests new scope, and dirB, which requests new & isolated scope. If dirA is applied after dirB, $compile reports an error ('Multiple directives [dirB, dirA] asking for new/isolated scope'), which is expected. However, if dirA is applied before dirB, e.g. because dirA has higher priority, then no error is thrown and dirA shares the isolated scope created for dirB. This is inconsistent and can break dirA, which might need to access to parent scope's members. This fix addresses this situation by throwing error regardless of order of directive application.

Closes #4402

… applied before isolated-scope directive

Given dirA, which requests new scope, and dirB, which requests new & isolated scope. If dirA is applied *after* dirB, $compile reports an error ('Multiple directives [dirB, dirA] asking for new/isolated scope'), which is expected. However, if dirA is applied *before* dirB, e.g. because dirA has high priority, then no error is thrown. This is the inconsistency. Plus, dirA ends up sharing the isolated scope created for dirB, which may break dirA because it might need to access to parent scope's members. This fix addresses this situation by throwing error regardless of order of directive application.

Closes angular#4402
@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!

@ghost ghost assigned petebacondarwin Dec 2, 2013
@petebacondarwin
Copy link
Contributor

Moving to 1.2.5 for further discussion:
I am not sure if this counts as a breaking change since it could well break applications that currently naively do have directives that "sort of" work right now and would break with an exception after this commit.

@caitp
Copy link
Contributor

caitp commented Dec 6, 2013

I mention this in 1307, but I realized that it seems to be a different issue, not throwing when two directives on the same element create a new (not isolate) scope. (this is after changing their plnkr to use 1.2.3).

So this might be a different issue, I'm not sure. In any case it seems like it's probably not desirable

@petebacondarwin
Copy link
Contributor

We can now land this since 1.3 can have breaking changes.

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

Successfully merging this pull request may close these issues.

$compile throws error inconsistently if isolated-scope directive's priority is set/unset
7 participants