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
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 9 additions & 5 deletions src/ng/compile.js
Original file line number Diff line number Diff line change
Expand Up @@ -3197,10 +3197,14 @@ function $CompileProvider($provide, $$sanitizeUriProvider) {
destination[scopeName] = parentGet(scope);
initialChanges[scopeName] = new SimpleChange(_UNINITIALIZED_VALUE, destination[scopeName]);

removeWatch = scope.$watch(parentGet, function parentValueWatchAction(newParentValue) {
var oldValue = destination[scopeName];
recordChanges(scopeName, newParentValue, oldValue);
destination[scopeName] = newParentValue;
removeWatch = scope.$watch(parentGet, function parentValueWatchAction(newValue, oldValue) {
if (newValue === oldValue) {
// If the new and old values are identical then this is the first time the watch has been triggered
// So instead we use the current value on the destination as the old value
oldValue = destination[scopeName];
}
recordChanges(scopeName, newValue, oldValue);
destination[scopeName] = newValue;
}, parentGet.literal);

removeWatchCollection.push(removeWatch);
Expand Down Expand Up @@ -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 :-)

previousValue = changes[key].previousValue;
}
// Store this change
Expand Down
25 changes: 25 additions & 0 deletions test/ng/compileSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -3742,6 +3742,31 @@ describe('$compile', function() {
});


it('should trigger `$onChanges` even if the inner value already equals the new outer value', function() {
var log = [];
function TestController() { }
TestController.prototype.$onChanges = function(change) { log.push(change); };

angular.module('my', [])
.component('c1', {
controller: TestController,
bindings: { 'prop1': '<' }
});

module('my');
inject(function($compile, $rootScope) {
element = $compile('<c1 prop1="val"></c1>')($rootScope);

$rootScope.$apply('val = 1');
expect(log.pop()).toEqual({prop1: jasmine.objectContaining({previousValue: undefined, currentValue: 1})});

element.isolateScope().$ctrl.prop1 = 2;
$rootScope.$apply('val = 2');
expect(log.pop()).toEqual({prop1: jasmine.objectContaining({previousValue: 1, currentValue: 2})});
});
});


it('should pass the original value as `previousValue` even if there were multiple changes in a single digest', function() {
var log = [];
function TestController() { }
Expand Down