diff --git a/benchmarks/parsed-expressions-bp/main.html b/benchmarks/parsed-expressions-bp/main.html index d7f65b742ce4..9884da94e6bc 100755 --- a/benchmarks/parsed-expressions-bp/main.html +++ b/benchmarks/parsed-expressions-bp/main.html @@ -31,6 +31,11 @@ +
  • + + +
  • +
  • @@ -134,6 +139,17 @@
  • +
  • + + + + + + + + +
  • +
  • diff --git a/src/ng/compile.js b/src/ng/compile.js index cff8fb38516c..f75cb7393f11 100644 --- a/src/ng/compile.js +++ b/src/ng/compile.js @@ -1725,7 +1725,7 @@ function $CompileProvider($provide, $$sanitizeUriProvider) { attrs[attrName], newIsolateScopeDirective.name); }; lastValue = isolateBindingContext[scopeName] = parentGet(scope); - var unwatch = scope.$watch($parse(attrs[attrName], function parentValueWatch(parentValue) { + var parentValueWatch = function parentValueWatch(parentValue) { if (!compare(parentValue, isolateBindingContext[scopeName])) { // we are out of sync and need to copy if (!compare(parentValue, lastValue)) { @@ -1737,7 +1737,9 @@ function $CompileProvider($provide, $$sanitizeUriProvider) { } } return lastValue = parentValue; - }), null, parentGet.literal); + }; + parentValueWatch.$stateful = true; + var unwatch = scope.$watch($parse(attrs[attrName], parentValueWatch), null, parentGet.literal); isolateScope.$on('$destroy', unwatch); break; diff --git a/src/ng/parse.js b/src/ng/parse.js index d09cc516a0db..eb6ef739edba 100644 --- a/src/ng/parse.js +++ b/src/ng/parse.js @@ -376,6 +376,10 @@ Lexer.prototype = { }; +function isConstant(exp) { + return exp.constant; +} + /** * @constructor */ @@ -493,7 +497,8 @@ Parser.prototype = { return extend(function(self, locals) { return fn(self, locals, right); }, { - constant:right.constant + constant:right.constant, + inputs: [right] }); }, @@ -505,11 +510,12 @@ Parser.prototype = { }); }, - binaryFn: function(left, fn, right) { + binaryFn: function(left, fn, right, isBranching) { return extend(function(self, locals) { return fn(self, locals, left, right); }, { - constant:left.constant && right.constant + constant: left.constant && right.constant, + inputs: !isBranching && [left, right] }); }, @@ -557,7 +563,9 @@ Parser.prototype = { } } - return function $parseFilter(self, locals) { + var inputs = [inputFn].concat(argsFn || []); + + return extend(function $parseFilter(self, locals) { var input = inputFn(self, locals); if (args) { args[0] = input; @@ -571,7 +579,10 @@ Parser.prototype = { } return fn(input); - }; + }, { + constant: !fn.$stateful && inputs.every(isConstant), + inputs: !fn.$stateful && inputs + }); }, expression: function() { @@ -588,9 +599,11 @@ Parser.prototype = { this.text.substring(0, token.index) + '] can not be assigned to', token); } right = this.ternary(); - return function $parseAssignment(scope, locals) { + return extend(function $parseAssignment(scope, locals) { return left.assign(scope, right(scope, locals), locals); - }; + }, { + inputs: [left, right] + }); } return left; }, @@ -615,7 +628,7 @@ Parser.prototype = { var left = this.logicalAND(); var token; while ((token = this.expect('||'))) { - left = this.binaryFn(left, token.fn, this.logicalAND()); + left = this.binaryFn(left, token.fn, this.logicalAND(), true); } return left; }, @@ -624,7 +637,7 @@ Parser.prototype = { var left = this.equality(); var token; if ((token = this.expect('&&'))) { - left = this.binaryFn(left, token.fn, this.logicalAND()); + left = this.binaryFn(left, token.fn, this.logicalAND(), true); } return left; }, @@ -759,7 +772,6 @@ Parser.prototype = { // This is used with json array declaration arrayDeclaration: function () { var elementFns = []; - var allConstant = true; if (this.peekToken().text !== ']') { do { if (this.peek(']')) { @@ -768,9 +780,6 @@ Parser.prototype = { } var elementFn = this.expression(); elementFns.push(elementFn); - if (!elementFn.constant) { - allConstant = false; - } } while (this.expect(',')); } this.consume(']'); @@ -783,13 +792,13 @@ Parser.prototype = { return array; }, { literal: true, - constant: allConstant + constant: elementFns.every(isConstant), + inputs: elementFns }); }, object: function () { var keys = [], valueFns = []; - var allConstant = true; if (this.peekToken().text !== '}') { do { if (this.peek('}')) { @@ -801,9 +810,6 @@ Parser.prototype = { this.consume(':'); var value = this.expression(); valueFns.push(value); - if (!value.constant) { - allConstant = false; - } } while (this.expect(',')); } this.consume('}'); @@ -816,7 +822,8 @@ Parser.prototype = { return object; }, { literal: true, - constant: allConstant + constant: valueFns.every(isConstant), + inputs: valueFns }); } }; @@ -1043,6 +1050,8 @@ function $ParseProvider() { parsedExpression = wrapSharedExpression(parsedExpression); parsedExpression.$$watchDelegate = parsedExpression.literal ? oneTimeLiteralWatchDelegate : oneTimeWatchDelegate; + } else if (parsedExpression.inputs) { + parsedExpression.$$watchDelegate = inputsWatchDelegate; } cache[cacheKey] = parsedExpression; @@ -1057,6 +1066,88 @@ function $ParseProvider() { } }; + function collectExpressionInputs(inputs, list) { + for (var i = 0, ii = inputs.length; i < ii; i++) { + var input = inputs[i]; + if (!input.constant) { + if (input.inputs) { + collectExpressionInputs(input.inputs, list); + } else if (list.indexOf(input) === -1) { // TODO(perf) can we do better? + list.push(input); + } + } + } + + return list; + } + + function expressionInputDirtyCheck(newValue, oldValueOfValue) { + + if (newValue == null || oldValueOfValue == null) { // null/undefined + return newValue === oldValueOfValue; + } + + if (typeof newValue === 'object') { + + // 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 + // be cheaply dirty-checked + newValue = newValue.valueOf(); + + if (typeof newValue === 'object') { + // objects/arrays are not supported - deep-watching them would be too expensive + return false; + } + + // fall-through to the primitive equality check + } + + //Primitive or NaN + return newValue === oldValueOfValue || (newValue !== newValue && oldValueOfValue !== oldValueOfValue); + } + + function inputsWatchDelegate(scope, listener, objectEquality, parsedExpression) { + var inputExpressions = parsedExpression.$$inputs || + (parsedExpression.$$inputs = collectExpressionInputs(parsedExpression.inputs, [])); + + var lastResult; + + if (inputExpressions.length === 1) { + var oldInputValue = expressionInputDirtyCheck; // init to something unique so that equals check fails + inputExpressions = inputExpressions[0]; + return scope.$watch(function expressionInputWatch(scope) { + var newInputValue = inputExpressions(scope); + if (!expressionInputDirtyCheck(newInputValue, oldInputValue)) { + lastResult = parsedExpression(scope); + oldInputValue = newInputValue && newInputValue.valueOf(); + } + return lastResult; + }, listener, objectEquality); + } + + var oldInputValueOfValues = []; + for (var i = 0, ii = inputExpressions.length; i < ii; i++) { + oldInputValueOfValues[i] = expressionInputDirtyCheck; // init to something unique so that equals check fails + } + + return scope.$watch(function expressionInputsWatch(scope) { + var changed = false; + + for (var i = 0, ii = inputExpressions.length; i < ii; i++) { + var newInputValue = inputExpressions[i](scope); + if (changed || (changed = !expressionInputDirtyCheck(newInputValue, oldInputValueOfValues[i]))) { + oldInputValueOfValues[i] = newInputValue && newInputValue.valueOf(); + } + } + + if (changed) { + lastResult = parsedExpression(scope); + } + + return lastResult; + }, listener, objectEquality); + } + function oneTimeWatchDelegate(scope, listener, objectEquality, parsedExpression) { var unwatch, lastValue; return unwatch = scope.$watch(function oneTimeWatch(scope) { @@ -1122,7 +1213,18 @@ function $ParseProvider() { // initial value is defined (for bind-once) return isDefined(value) ? result : value; }; - fn.$$watchDelegate = parsedExpression.$$watchDelegate; + + // Propagate $$watchDelegates other then inputsWatchDelegate + if (parsedExpression.$$watchDelegate && + parsedExpression.$$watchDelegate !== inputsWatchDelegate) { + fn.$$watchDelegate = parsedExpression.$$watchDelegate; + } else if (!interceptorFn.$stateful) { + // If there is an interceptor, but no watchDelegate then treat the interceptor like + // we treat filters - it is assumed to be a pure function unless flagged with $stateful + fn.$$watchDelegate = inputsWatchDelegate; + fn.inputs = [parsedExpression]; + } + return fn; } }]; diff --git a/src/ng/rootScope.js b/src/ng/rootScope.js index 77cba19968ab..34e69b1bd325 100644 --- a/src/ng/rootScope.js +++ b/src/ng/rootScope.js @@ -515,6 +515,8 @@ function $RootScopeProvider(){ * de-registration function is executed, the internal watch operation is terminated. */ $watchCollection: function(obj, listener) { + $watchCollectionInterceptor.$stateful = true; + var self = this; // the current value, updated on each dirty-check run var newValue; diff --git a/test/ng/directive/ngRepeatSpec.js b/test/ng/directive/ngRepeatSpec.js index 6ee1cbd06bd8..609d232b6f0f 100644 --- a/test/ng/directive/ngRepeatSpec.js +++ b/test/ng/directive/ngRepeatSpec.js @@ -867,12 +867,15 @@ describe('ngRepeat', function() { // This creates one item, but it has no parent so we can't get to it $rootScope.items = [1, 2]; $rootScope.$apply(); + expect(logs).toContain(1); + expect(logs).toContain(2); + logs.length = 0; // This cleans up to prevent memory leak $rootScope.items = []; $rootScope.$apply(); expect(angular.mock.dump(element)).toBe(''); - expect(logs).toEqual([1, 2, 1, 2]); + expect(logs.length).toBe(0); })); @@ -894,12 +897,15 @@ describe('ngRepeat', function() { // This creates one item, but it has no parent so we can't get to it $rootScope.items = [1, 2]; $rootScope.$apply(); + expect(logs).toContain(1); + expect(logs).toContain(2); + logs.length = 0; // This cleans up to prevent memory leak $rootScope.items = []; $rootScope.$apply(); expect(sortedHtml(element)).toBe('--'); - expect(logs).toEqual([1, 2, 1, 2]); + expect(logs.length).toBe(0); })); diff --git a/test/ng/parseSpec.js b/test/ng/parseSpec.js index 122422d4e302..b77a76b7ad5a 100644 --- a/test/ng/parseSpec.js +++ b/test/ng/parseSpec.js @@ -1304,6 +1304,240 @@ describe('parser', function() { }); }); + + describe('watched $parse expressions', function() { + + it('should respect short-circuiting AND if it could have side effects', function() { + var bCalled = 0; + scope.b = function() { bCalled++; }; + + scope.$watch("a && b()"); + scope.$digest(); + scope.$digest(); + expect(bCalled).toBe(0); + + scope.a = true; + scope.$digest(); + expect(bCalled).toBe(1); + scope.$digest(); + expect(bCalled).toBe(2); + }); + + it('should respect short-circuiting OR if it could have side effects', function() { + var bCalled = false; + scope.b = function() { bCalled = true; }; + + scope.$watch("a || b()"); + scope.$digest(); + expect(bCalled).toBe(true); + + bCalled = false; + scope.a = true; + scope.$digest(); + expect(bCalled).toBe(false); + }); + + it('should respect the branching ternary operator if it could have side effects', function() { + var bCalled = false; + scope.b = function() { bCalled = true; }; + + scope.$watch("a ? b() : 1"); + scope.$digest(); + expect(bCalled).toBe(false); + + scope.a = true; + scope.$digest(); + expect(bCalled).toBe(true); + }); + + it('should not invoke filters unless the input/arguments change', function() { + var filterCalled = false; + $filterProvider.register('foo', valueFn(function(input) { + filterCalled = true; + return input; + })); + + scope.$watch("a | foo:b:1"); + scope.a = 0; + scope.$digest(); + expect(filterCalled).toBe(true); + + filterCalled = false; + scope.$digest(); + expect(filterCalled).toBe(false); + + scope.a++; + scope.$digest(); + expect(filterCalled).toBe(true); + }); + + it('should invoke filters if they are marked as having $stateful', function() { + var filterCalled = false; + $filterProvider.register('foo', valueFn(extend(function(input) { + filterCalled = true; + return input; + }, {$stateful: true}))); + + scope.$watch("a | foo:b:1"); + scope.a = 0; + scope.$digest(); + expect(filterCalled).toBe(true); + + filterCalled = false; + scope.$digest(); + expect(filterCalled).toBe(true); + }); + + it('should not invoke interceptorFns unless the input changes', inject(function($parse) { + var called = false; + function interceptor(v) { + called = true; + return v; + } + scope.$watch($parse("a", interceptor)); + scope.a = scope.b = 0; + scope.$digest(); + expect(called).toBe(true); + + called = false; + scope.$digest(); + expect(called).toBe(false); + + scope.a++; + scope.$digest(); + expect(called).toBe(true); + })); + + it('should treat filters with constant input as constants', inject(function($parse) { + var filterCalls = 0; + $filterProvider.register('foo', valueFn(function(input) { + filterCalls++; + return input; + })); + + var parsed = $parse('{x: 1} | foo:1'); + + expect(parsed.constant).toBe(true); + + var watcherCalls = 0; + scope.$watch(parsed, function(input) { + expect(input).toEqual({x:1}); + watcherCalls++; + }); + + scope.$digest(); + expect(filterCalls).toBe(1); + expect(watcherCalls).toBe(1); + + scope.$digest(); + expect(filterCalls).toBe(1); + expect(watcherCalls).toBe(1); + })); + + it("should always reevaluate filters with non-primitive input that doesn't support valueOf()", + inject(function($parse) { + var filterCalls = 0; + $filterProvider.register('foo', valueFn(function(input) { + filterCalls++; + return input; + })); + + var parsed = $parse('obj | foo'); + var obj = scope.obj = {}; + + var watcherCalls = 0; + scope.$watch(parsed, function(input) { + expect(input).toBe(obj); + watcherCalls++; + }); + + scope.$digest(); + expect(filterCalls).toBe(2); + expect(watcherCalls).toBe(1); + + scope.$digest(); + expect(filterCalls).toBe(3); + expect(watcherCalls).toBe(1); + })); + + it("should not reevaluate filters with non-primitive input that does support valueOf()", + inject(function($parse) { + var filterCalls = 0; + $filterProvider.register('foo', valueFn(function(input) { + filterCalls++; + return input; + })); + + var parsed = $parse('date | foo'); + var date = scope.date = new Date(); + + var watcherCalls = 0; + scope.$watch(parsed, function(input) { + expect(input).toBe(date); + watcherCalls++; + }); + + scope.$digest(); + expect(filterCalls).toBe(1); + expect(watcherCalls).toBe(1); + + scope.$digest(); + expect(filterCalls).toBe(1); + expect(watcherCalls).toBe(1); + })); + + it("should reevaluate filters with non-primitive input that does support valueOf() when" + + "valueOf() value changes", inject(function($parse) { + var filterCalls = 0; + $filterProvider.register('foo', valueFn(function(input) { + filterCalls++; + return input; + })); + + var parsed = $parse('date | foo'); + var date = scope.date = new Date(); + + var watcherCalls = 0; + scope.$watch(parsed, function(input) { + expect(input).toBe(date); + watcherCalls++; + }); + + scope.$digest(); + expect(filterCalls).toBe(1); + expect(watcherCalls).toBe(1); + + date.setYear(1901); + + scope.$digest(); + expect(filterCalls).toBe(2); + expect(watcherCalls).toBe(1); + })); + + it('should invoke interceptorFns if they are flagged as having $stateful', + inject(function($parse) { + var called = false; + function interceptor() { + called = true; + } + interceptor.$stateful = true; + + scope.$watch($parse("a", interceptor)); + scope.a = 0; + scope.$digest(); + expect(called).toBe(true); + + called = false; + scope.$digest(); + expect(called).toBe(true); + + scope.a++; + called = false; + scope.$digest(); + expect(called).toBe(true); + })); + }); + describe('locals', function() { it('should expose local variables', inject(function($parse) { expect($parse('a')({a: 0}, {a: 1})).toEqual(1);