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

fix($compile): check clashing directives before compilation #4217

Closed
wants to merge 1 commit into from

Conversation

jankuca
Copy link
Contributor

@jankuca jankuca commented Oct 1, 2013

Clashing directive check was not working correctly when two directives requested 'element' transclusion because one would start a new compilation (a recursive sub procedure) with any duplicate info being reset.

This PR moves the checking to its own loop which is executed before the actual compilation.

@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!

@vojtajina
Copy link
Contributor

This might solve #3307

@IgorMinar
Copy link
Contributor

I'm taking over this one since vojta is out today.

@ghost ghost assigned IgorMinar Oct 10, 2013
@IgorMinar
Copy link
Contributor

This strategy doesn't work for cases when directives are lazily added onto the node from compile functions or via the replace option.

IgorMinar added a commit to IgorMinar/angular.js that referenced this pull request Oct 11, 2013
Issue an error and abort compilation when two directives that ask for transclusion are found
on a single element. This configuration is not supported and we previously failed to issue
the error because in the case of element transclusion the compilation is re-started and this
caused the compilation context to be lost.

The ngRepeat directive has been special-cased to bypass this warning because it knows how to
handle this scenario internally.

This is not an ideal solution to the problem of multiple transclusions per element, we are
hoping to have this configuration supported by the compiler in the future. See angular#4357.

Closes angular#3893
Closes angular#4217
Closes angular#3307
@IgorMinar
Copy link
Contributor

I'm closing this PR for now since it's obsolete.

@IgorMinar IgorMinar closed this Oct 11, 2013
IgorMinar added a commit to IgorMinar/angular.js that referenced this pull request Oct 12, 2013
Issue an error and abort compilation when two directives that ask for transclusion are found
on a single element. This configuration is not supported and we previously failed to issue
the error because in the case of element transclusion the compilation is re-started and this
caused the compilation context to be lost.

The ngRepeat directive has been special-cased to bypass this warning because it knows how to
handle this scenario internally.

This is not an ideal solution to the problem of multiple transclusions per element, we are
hoping to have this configuration supported by the compiler in the future. See angular#4357.

Closes angular#3893
Closes angular#4217
Closes angular#3307
IgorMinar added a commit that referenced this pull request Oct 12, 2013
Issue an error and abort compilation when two directives that ask for transclusion are found
on a single element. This configuration is not supported and we previously failed to issue
the error because in the case of element transclusion the compilation is re-started and this
caused the compilation context to be lost.

The ngRepeat directive has been special-cased to bypass this warning because it knows how to
handle this scenario internally.

This is not an ideal solution to the problem of multiple transclusions per element, we are
hoping to have this configuration supported by the compiler in the future. See #4357.

Closes #3893
Closes #4217
Closes #3307
jamesdaily pushed a commit to jamesdaily/angular.js that referenced this pull request Jan 27, 2014
Issue an error and abort compilation when two directives that ask for transclusion are found
on a single element. This configuration is not supported and we previously failed to issue
the error because in the case of element transclusion the compilation is re-started and this
caused the compilation context to be lost.

The ngRepeat directive has been special-cased to bypass this warning because it knows how to
handle this scenario internally.

This is not an ideal solution to the problem of multiple transclusions per element, we are
hoping to have this configuration supported by the compiler in the future. See angular#4357.

Closes angular#3893
Closes angular#4217
Closes angular#3307
jamesdaily pushed a commit to jamesdaily/angular.js that referenced this pull request Jan 27, 2014
Issue an error and abort compilation when two directives that ask for transclusion are found
on a single element. This configuration is not supported and we previously failed to issue
the error because in the case of element transclusion the compilation is re-started and this
caused the compilation context to be lost.

The ngRepeat directive has been special-cased to bypass this warning because it knows how to
handle this scenario internally.

This is not an ideal solution to the problem of multiple transclusions per element, we are
hoping to have this configuration supported by the compiler in the future. See angular#4357.

Closes angular#3893
Closes angular#4217
Closes angular#3307
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.

4 participants