From c175619d0f0acb82d24343f538e6ada12d99aa13 Mon Sep 17 00:00:00 2001 From: Jason Bedard Date: Thu, 20 Oct 2016 01:59:56 -0700 Subject: [PATCH 1/4] fix($parse): allow watching array/object literal values - the content of array/object literals is now watched for changes - if the content changes then a new array/object will be constructed --- src/ng/parse.js | 8 +++--- test/ng/parseSpec.js | 68 ++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 72 insertions(+), 4 deletions(-) diff --git a/src/ng/parse.js b/src/ng/parse.js index 3e2661e16f6d..91cfcf9d4ba3 100644 --- a/src/ng/parse.js +++ b/src/ng/parse.js @@ -1786,13 +1786,13 @@ function $ParseProvider() { } } - function expressionInputDirtyCheck(newValue, oldValueOfValue) { + function expressionInputDirtyCheck(newValue, oldValueOfValue, compareObjectIdentity) { if (newValue == null || oldValueOfValue == null) { // null/undefined return newValue === oldValueOfValue; } - if (typeof newValue === 'object') { + if (typeof newValue === 'object' && !compareObjectIdentity) { // attempt to convert the value to a primitive type // TODO(docs): add a note to docs that by implementing valueOf even objects and arrays can @@ -1821,7 +1821,7 @@ function $ParseProvider() { inputExpressions = inputExpressions[0]; return scope.$watch(function expressionInputWatch(scope) { var newInputValue = inputExpressions(scope); - if (!expressionInputDirtyCheck(newInputValue, oldInputValueOf)) { + if (!expressionInputDirtyCheck(newInputValue, oldInputValueOf, parsedExpression.literal)) { lastResult = parsedExpression(scope, undefined, undefined, [newInputValue]); oldInputValueOf = newInputValue && getValueOf(newInputValue); } @@ -1841,7 +1841,7 @@ function $ParseProvider() { for (var i = 0, ii = inputExpressions.length; i < ii; i++) { var newInputValue = inputExpressions[i](scope); - if (changed || (changed = !expressionInputDirtyCheck(newInputValue, oldInputValueOfValues[i]))) { + if (changed || (changed = !expressionInputDirtyCheck(newInputValue, oldInputValueOfValues[i], parsedExpression.literal))) { oldInputValues[i] = newInputValue; oldInputValueOfValues[i] = newInputValue && getValueOf(newInputValue); } diff --git a/test/ng/parseSpec.js b/test/ng/parseSpec.js index 6af4671b264f..171341e1bdf8 100644 --- a/test/ng/parseSpec.js +++ b/test/ng/parseSpec.js @@ -3117,6 +3117,74 @@ describe('parser', function() { scope.$digest(); expect(objB.value).toBe(scope.input); })); + + it('should support watching literals', inject(function($parse) { + var lastVal = NaN; + var callCount = 0; + var listener = function(val) { callCount++; lastVal = val; }; + + scope.$watch('{val: val}', listener); + + scope.$apply('val = 1'); + expect(callCount).toBe(1); + expect(lastVal).toEqual({val: 1}); + + scope.$apply('val = []'); + expect(callCount).toBe(2); + expect(lastVal).toEqual({val: []}); + + scope.$apply('val = []'); + expect(callCount).toBe(3); + expect(lastVal).toEqual({val: []}); + + scope.$apply('val = {}'); + expect(callCount).toBe(4); + expect(lastVal).toEqual({val: {}}); + })); + + it('should only watch the direct inputs to literals', inject(function($parse) { + var lastVal = NaN; + var callCount = 0; + var listener = function(val) { callCount++; lastVal = val; }; + + scope.$watch('{val: val}', listener); + + scope.$apply('val = 1'); + expect(callCount).toBe(1); + expect(lastVal).toEqual({val: 1}); + + scope.$apply('val = [2]'); + expect(callCount).toBe(2); + expect(lastVal).toEqual({val: [2]}); + + scope.$apply('val.push(3)'); + expect(callCount).toBe(2); + + scope.$apply('val.length = 0'); + expect(callCount).toBe(2); + })); + + it('should only watch the direct inputs to nested literals', inject(function($parse) { + var lastVal = NaN; + var callCount = 0; + var listener = function(val) { callCount++; lastVal = val; }; + + scope.$watch('[{val: [val]}]', listener); + + scope.$apply('val = 1'); + expect(callCount).toBe(1); + expect(lastVal).toEqual([{val: [1]}]); + + scope.$apply('val = [2]'); + expect(callCount).toBe(2); + expect(lastVal).toEqual([{val: [[2]]}]); + + scope.$apply('val.push(3)'); + expect(callCount).toBe(2); + + scope.$apply('val.length = 0'); + expect(callCount).toBe(2); + })); }); describe('locals', function() { From 65ff861f99e2db0489c23fce5813e0bc76b27c44 Mon Sep 17 00:00:00 2001 From: Jason Bedard Date: Thu, 20 Oct 2016 02:06:32 -0700 Subject: [PATCH 2/4] perf($compile): update one-way bindings to no longer use deepWatch for literals BREAKING CHANGE: Previously when a literal value was passed into a directive/component via one-way binding it would be watched with a deep watcher. For example, for ``, a new instance of the array would be passed into the directive/component (and trigger $onChanges) not only if `a` changed but also if any sub property of `a` changed such as `a.b` or `a.b.c.d.e` etc. This also means a new but equal value for `a` would NOT trigger such a change. Now literal values use a non-deep watch similar to other directive/component one-way bindings. Changes are only trigger when the value itself changes. Avoiding deep watchers for array/object literals will improve watcher performance of all literals passed as one-way bindings, especially those containing references to large/complex objects. --- src/ng/compile.js | 2 +- test/ng/compileSpec.js | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/ng/compile.js b/src/ng/compile.js index bc1cae5bfca6..0887a7086b10 100644 --- a/src/ng/compile.js +++ b/src/ng/compile.js @@ -3522,7 +3522,7 @@ function $CompileProvider($provide, $$sanitizeUriProvider) { } recordChanges(scopeName, newValue, oldValue); destination[scopeName] = newValue; - }, deepWatch); + }); removeWatchCollection.push(removeWatch); break; diff --git a/test/ng/compileSpec.js b/test/ng/compileSpec.js index 544baf8a74ca..1a05d8ff232e 100755 --- a/test/ng/compileSpec.js +++ b/test/ng/compileSpec.js @@ -5676,7 +5676,7 @@ describe('$compile', function() { })); - it('should deep-watch array literals', inject(function() { + it('should watch input values to array literals', inject(function() { $rootScope.name = 'georgios'; $rootScope.obj = {name: 'pete'}; compile('
'); @@ -5690,7 +5690,7 @@ describe('$compile', function() { })); - it('should deep-watch object literals', inject(function() { + it('should watch input values object literals', inject(function() { $rootScope.name = 'georgios'; $rootScope.obj = {name: 'pete'}; compile('
'); From 3d81ceed381a78f771a148de742bf1e2413786a6 Mon Sep 17 00:00:00 2001 From: Jason Bedard Date: Fri, 27 Jan 2017 00:49:53 -0800 Subject: [PATCH 3/4] fixup! add tests verifying one-way watch does not use deep-equals --- test/ng/compileSpec.js | 72 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 72 insertions(+) diff --git a/test/ng/compileSpec.js b/test/ng/compileSpec.js index 1a05d8ff232e..35e174845ee4 100755 --- a/test/ng/compileSpec.js +++ b/test/ng/compileSpec.js @@ -4259,6 +4259,78 @@ describe('$compile', function() { }); + it('should trigger `$onChanges` for literal expressions when expression input value changes (simple 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('')($rootScope); + + $rootScope.$apply('val = 1'); + expect(log.pop()).toEqual({prop1: jasmine.objectContaining({previousValue: [undefined], currentValue: [1]})}); + + $rootScope.$apply('val = 2'); + expect(log.pop()).toEqual({prop1: jasmine.objectContaining({previousValue: [1], currentValue: [2]})}); + }); + }); + + + it('should trigger `$onChanges` for literal expressions when expression input value changes (complex 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('')($rootScope); + + $rootScope.$apply('val = [1]'); + expect(log.pop()).toEqual({prop1: jasmine.objectContaining({previousValue: [undefined], currentValue: [[1]]})}); + + $rootScope.$apply('val = [2]'); + expect(log.pop()).toEqual({prop1: jasmine.objectContaining({previousValue: [[1]], currentValue: [[2]]})}); + }); + }); + + + it('should trigger `$onChanges` for literal expressions when expression input value changes instances, even when equal', 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('')($rootScope); + + $rootScope.$apply('val = [1]'); + expect(log.pop()).toEqual({prop1: jasmine.objectContaining({previousValue: [undefined], currentValue: [[1]]})}); + + $rootScope.$apply('val = [1]'); + expect(log.pop()).toEqual({prop1: jasmine.objectContaining({previousValue: [[1]], currentValue: [[1]]})}); + }); + }); + + it('should pass the original value as `previousValue` even if there were multiple changes in a single digest', function() { var log = []; function TestController() { } From 0706760acabae47217f1741c36ce96be74c8ddc0 Mon Sep 17 00:00:00 2001 From: Jason Bedard Date: Fri, 27 Jan 2017 00:52:42 -0800 Subject: [PATCH 4/4] fixup! rename deepWatch variable --- src/ng/compile.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/ng/compile.js b/src/ng/compile.js index 0887a7086b10..f26c60f57772 100644 --- a/src/ng/compile.js +++ b/src/ng/compile.js @@ -3508,14 +3508,14 @@ function $CompileProvider($provide, $$sanitizeUriProvider) { if (optional && !attrs[attrName]) break; parentGet = $parse(attrs[attrName]); - var deepWatch = parentGet.literal; + var isLiteral = parentGet.literal; var initialValue = destination[scopeName] = parentGet(scope); initialChanges[scopeName] = new SimpleChange(_UNINITIALIZED_VALUE, destination[scopeName]); removeWatch = scope.$watch(parentGet, function parentValueWatchAction(newValue, oldValue) { if (oldValue === newValue) { - if (oldValue === initialValue || (deepWatch && equals(oldValue, initialValue))) { + if (oldValue === initialValue || (isLiteral && equals(oldValue, initialValue))) { return; } oldValue = initialValue;