This repository has been archived by the owner on Apr 12, 2024. It is now read-only.
fix($compile): attribute bindings should not break due to terminal directives #4649
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Thanks for the PR!
If you need to make changes to your pull request, you can update the commit with Thanks again for your help! |
ghost
assigned 
|
LGTM |
| attr.$set(name, value); | ||
| }); | ||
| }) | ||
| // we need to interpolate again, in case the attribute value has been updated |
There was a problem hiding this comment.
This comment worries me. That we have to interpolate twice seems wrong - this seems to catch if another directive changes an attribute that previously had an interpolation but what happens if another directive adds an interpolation to an attribute that didn't have interpolation before? Won't it be missed?
There was a problem hiding this comment.
yes it will be. I don't think that there is a way around it.
if an attribute doesn't have interpolation at compile time then we won't establish a databinding for that attribute.
…rectives Recently we changed the priority of attribute interpolation directive to -100 to ensure that it executes early in the post linking phase. This causes issues with when terminal directives are placed on elements with attribute bindings because the terminal directive will usually have 0 or higher priority which results in attr interpolation directive not being applied to the element. To fix this issue I'm switching the priority back to 100 and making moving the binding setup into the pre-linking function. This means that: - terminal directives with priority lower than 100 will not affect the attribute binding - if a directive wants to add or alter bindings it can do so in the pre-linking phase, as long as the priority of this directive is more than 100 - all post-linking functions will execute after the attribute binding has been set up - all pre-linking functions with directive priority lower than 100 will execute after the attribute bindings have been setup BREAKING CHANGE: the attribute interpolation (binding) executes as a directive with priority 100 and the binding is set up in the pre-linking phase. It used to be that the priority was -100 in rc.2 (100 before rc.2) and that the binding was setup in the post-linking phase. Closes angular#4525 Closes angular#4528 Closes angular#4649
jamesdaily
pushed a commit
to jamesdaily/angular.js
that referenced
this pull request
Jan 27, 2014
…rectives Recently we changed the priority of attribute interpolation directive to -100 to ensure that it executes early in the post linking phase. This causes issues with when terminal directives are placed on elements with attribute bindings because the terminal directive will usually have 0 or higher priority which results in attr interpolation directive not being applied to the element. To fix this issue I'm switching the priority back to 100 and making moving the binding setup into the pre-linking function. This means that: - terminal directives with priority lower than 100 will not affect the attribute binding - if a directive wants to add or alter bindings it can do so in the pre-linking phase, as long as the priority of this directive is more than 100 - all post-linking functions will execute after the attribute binding has been set up - all pre-linking functions with directive priority lower than 100 will execute after the attribute bindings have been setup BREAKING CHANGE: the attribute interpolation (binding) executes as a directive with priority 100 and the binding is set up in the pre-linking phase. It used to be that the priority was -100 in rc.2 (100 before rc.2) and that the binding was setup in the post-linking phase. Closes angular#4525 Closes angular#4528 Closes angular#4649
jamesdaily
pushed a commit
to jamesdaily/angular.js
that referenced
this pull request
Jan 27, 2014
…rectives Recently we changed the priority of attribute interpolation directive to -100 to ensure that it executes early in the post linking phase. This causes issues with when terminal directives are placed on elements with attribute bindings because the terminal directive will usually have 0 or higher priority which results in attr interpolation directive not being applied to the element. To fix this issue I'm switching the priority back to 100 and making moving the binding setup into the pre-linking function. This means that: - terminal directives with priority lower than 100 will not affect the attribute binding - if a directive wants to add or alter bindings it can do so in the pre-linking phase, as long as the priority of this directive is more than 100 - all post-linking functions will execute after the attribute binding has been set up - all pre-linking functions with directive priority lower than 100 will execute after the attribute bindings have been setup BREAKING CHANGE: the attribute interpolation (binding) executes as a directive with priority 100 and the binding is set up in the pre-linking phase. It used to be that the priority was -100 in rc.2 (100 before rc.2) and that the binding was setup in the post-linking phase. Closes angular#4525 Closes angular#4528 Closes angular#4649
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Recently we changed the priority of attribute interpolation directive to -100
to ensure that it executes early in the post linking phase. This causes issues
with when terminal directives are placed on elements with attribute bindings
because the terminal directive will usually have 0 or higher priority which
results in attr interpolation directive not being applied to the element.
To fix this issue I'm switching the priority back to 100 and making moving the
binding setup into the pre-linking function.
This means that:
binding
phase, as long as the priority of this directive is more than 100
set up
after the attribute bindings have been setup
BREAKING CHANGE: the attribute interpolation (binding) executes as a directive
with priority 100 and the binding is set up in the pre-linking phase. It used
to be that the priority was -100 in rc.2 (100 before rc.2) and that the binding
was setup in the post-linking phase.
Closes #4525
Closes #4528