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

fix($compile): still trigger $onChanges even if the inner value alr… #14406

Closed

Conversation

petebacondarwin
Copy link
Contributor

What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)

fix

What is the current behavior? (You can also link to an open issue here)

the $onChanges hook is not getting called if the value on the controller is already set to the new value

A2 does trigger this value - see http://plnkr.co/edit/V15jwFA7e5zsWgLVLDKf?p=preview

What is the new behavior (if this is a feature change)?

now the hook is triggered

Does this PR introduce a breaking change?

no

Please check if the PR fulfills these requirements

Other information:

…eady matches the new value

@@ -3233,7 +3237,7 @@ function $CompileProvider($provide, $$sanitizeUriProvider) {
onChangesQueue.push(triggerOnChangesHook);
}
// If the has been a change on this property already then we need to reuse the previous value
if (changes[key]) {
if (isDefined(changes[key])) {
Copy link
Member

Choose a reason for hiding this comment

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

Is this necessary ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It seemed safer

Copy link
Contributor Author

Choose a reason for hiding this comment

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

But I guess since the property value must be undefined or an object it is not strictly necessary

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Feel free to do a refactor commit to remove it :-)

petebacondarwin added a commit that referenced this pull request Apr 10, 2016
petebacondarwin added a commit that referenced this pull request Apr 10, 2016
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.

3 participants