From b712105258092dc151f8a5cd3ac17917cf097377 Mon Sep 17 00:00:00 2001 From: rodyhaddad Date: Tue, 25 Feb 2014 12:27:20 -0500 Subject: [PATCH 01/11] feat(Scope): add `$watchGroup` method for observing a set of expressions Given an array of expressions, if any one expression changes then the listener function fires with an arrays of old and new values. $scope.watchGroup([expression1, expression2, expression3], function(newVals, oldVals) { // newVals and oldVals are arrays of values corresponding to expression1..3 ... }); Port of angular/angular.dart@a3c31ce1dddb4423faa316cb144568f3fc28b1a9 --- src/ng/rootScope.js | 62 +++++++++++++++++++++++++++++- test/ng/rootScopeSpec.js | 83 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 144 insertions(+), 1 deletion(-) diff --git a/src/ng/rootScope.js b/src/ng/rootScope.js index e7bdc4142923..a9ed64620480 100644 --- a/src/ng/rootScope.js +++ b/src/ng/rootScope.js @@ -354,6 +354,66 @@ function $RootScopeProvider(){ }; }, + /** + * @ngdoc method + * @name $rootScope.Scope#$watchGroup + * @function + * + * @description + * A variant of {@link ng.$rootScope.Scope#$watch $watch()} where it watches an array of `watchExpressions`. + * If any one expression in the collection changes the `listener` is executed. + * + * - The items in the `watchCollection` array are observed via standard $watch operation and are examined on every + * call to $digest() to see if any items changes. + * - The `listener` is called whenever any expression in the `watchExpressions` array changes. + * + * @param {Array.} watchExpressions Array of expressions that will be individually + * watched using {@link ng.$rootScope.Scope#$watch $watch()} + * + * @param {function(newValues, oldValues, scope)} listener Callback called whenever the return value of any + * expression in `watchExpressions` changes + * The `newValues` array contains the current values of the `watchExpressions`, with the indexes matching + * those of `watchExpression` + * and the `oldValues` array contains the previous values of the `watchExpressions`, with the indexes matching + * those of `watchExpression` + * The `scope` refers to the current scope. + * + * @returns {function()} Returns a de-registration function for all listeners. + */ + $watchGroup: function(watchExpressions, listener) { + var oldValues = new Array(watchExpressions.length), + newValues = new Array(watchExpressions.length); + + if (watchExpressions.length === 1) { + // Special case size of one + return this.$watch(watchExpressions[0], function (value, oldValue, scope) { + newValues[0] = value; + oldValues[0] = oldValue; + listener.call(this, newValues, oldValues, scope); + }); + } + var deregisterFns = [], + changeCount = 0; + + forEach(watchExpressions, function (expr, i) { + deregisterFns.push(this.$watch(expr, function (value, oldValue) { + newValues[i] = value; + oldValues[i] = oldValue; + changeCount++; + })); + }, this); + + deregisterFns.push(this.$watch(function () {return changeCount;}, function (c, o, scope) { + listener.call(this, newValues, oldValues, scope); + })); + + return function deregisterWatchGroup() { + forEach(deregisterFns, function (fn) { + fn(); + }); + }; + }, + /** * @ngdoc method @@ -756,7 +816,7 @@ function $RootScopeProvider(){ // prevent NPEs since these methods have references to properties we nulled out this.$destroy = this.$digest = this.$apply = noop; - this.$on = this.$watch = function() { return noop; }; + this.$on = this.$watch = this.$watchGroup = function() { return noop; }; }, /** diff --git a/test/ng/rootScopeSpec.js b/test/ng/rootScopeSpec.js index 01b5d1bdfdd4..77411e596eb3 100644 --- a/test/ng/rootScopeSpec.js +++ b/test/ng/rootScopeSpec.js @@ -777,6 +777,89 @@ describe('Scope', function() { }); }); + describe('$watchGroup', function() { + var scope; + beforeEach(inject(function($rootScope) { + scope = $rootScope.$new(); + })); + + + it('should treat set of 1 as direct watch', function() { + var lastValues = ['foo']; + var log = ''; + var clean = scope.$watchGroup(['a'], function(values, oldValues, s) { + log += values.join(',') + ';'; + expect(s).toBe(scope); + expect(oldValues).toEqual(lastValues); + lastValues = values.slice(); + }); + + scope.a = 'foo'; + scope.$digest(); + expect(log).toEqual('foo;'); + + scope.$digest(); + expect(log).toEqual('foo;'); + + scope.a = 'bar'; + scope.$digest(); + expect(log).toEqual('foo;bar;'); + + clean(); + scope.a = 'xxx'; + scope.$digest(); + expect(log).toEqual('foo;bar;'); + }); + + + it('should detect a change to any one in a set', function() { + var lastValues = ['foo', 'bar']; + var log = ''; + + scope.$watchGroup(['a', 'b'], function(values, oldValues, s) { + log += values.join(',') + ';'; + expect(s).toBe(scope); + expect(oldValues).toEqual(lastValues); + lastValues = values.slice(); + }); + + scope.a = 'foo'; + scope.b = 'bar'; + scope.$digest(); + expect(log).toEqual('foo,bar;'); + + log = ''; + scope.$digest(); + expect(log).toEqual(''); + + scope.a = 'a'; + scope.$digest(); + expect(log).toEqual('a,bar;'); + + log = ''; + scope.a = 'A'; + scope.b = 'B'; + scope.$digest(); + expect(log).toEqual('A,B;'); + }); + + + it('should not call watch action fn when watchGroup was deregistered', function() { + var lastValues = ['foo', 'bar']; + var log = ''; + var deregister = scope.$watchGroup(['a', 'b'], function(values, oldValues, s) { + log += values.join(',') + ';'; + expect(s).toBe(scope); + expect(oldValues).toEqual(lastValues); + lastValues = values.slice(); + }); + + deregister(); + scope.a = 'xxx'; + scope.$digest(); + expect(log).toEqual(''); + }); + }); describe('$destroy', function() { var first = null, middle = null, last = null, log = null; From fd8f98294d1e6b9d12234dfedfd8f4e8593320fb Mon Sep 17 00:00:00 2001 From: rodyhaddad Date: Thu, 6 Mar 2014 15:57:24 -0500 Subject: [PATCH 02/11] refactor($interpolate): split .parts into .expressions and .separators BREAKING CHANGE: the function returned by $interpolate no longer has a `.parts` array set on it. It has been replaced by two arrays: * `.expressions`, an array of the expressions in the interpolated text. The expressions are parsed with $parse, with an extra layer converting them to strings when computed * `.separators`, an array of strings representing the separations between interpolations in the text. This array is **always** 1 item longer than the `.expressions` array for easy merging with it --- src/ng/interpolate.js | 98 ++++++++++++++++++++++++-------------- src/ngScenario/Scenario.js | 4 +- test/ng/interpolateSpec.js | 96 +++++++++++++++++++++---------------- 3 files changed, 118 insertions(+), 80 deletions(-) diff --git a/src/ng/interpolate.js b/src/ng/interpolate.js index 6b61e56f2a70..e889133eb018 100644 --- a/src/ng/interpolate.js +++ b/src/ng/interpolate.js @@ -125,32 +125,36 @@ function $InterpolateProvider() { var startIndex, endIndex, index = 0, - parts = [], - length = text.length, + separators = [], + expressions = [], + textLength = text.length, hasInterpolation = false, + hasText = false, fn, exp, concat = []; - while(index < length) { + while(index < textLength) { if ( ((startIndex = text.indexOf(startSymbol, index)) != -1) && ((endIndex = text.indexOf(endSymbol, startIndex + startSymbolLength)) != -1) ) { - (index != startIndex) && parts.push(text.substring(index, startIndex)); - parts.push(fn = $parse(exp = text.substring(startIndex + startSymbolLength, endIndex))); + if (index !== startIndex) hasText = true; + separators.push(text.substring(index, startIndex)); + expressions.push(fn = interpolateParse(exp = text.substring(startIndex + startSymbolLength, endIndex))); fn.exp = exp; index = endIndex + endSymbolLength; hasInterpolation = true; } else { - // we did not find anything, so we have to add the remainder to the parts array - (index != length) && parts.push(text.substring(index)); - index = length; + // we did not find an interpolation, so we have to add the remainder to the separators array + if (index !== textLength) { + hasText = true; + separators.push(text.substring(index)); + } + break; } } - if (!(length = parts.length)) { - // we added, nothing, must have been an empty string. - parts.push(''); - length = 1; + if (separators.length === expressions.length) { + separators.push(''); } // Concatenating expressions makes it hard to reason about whether some combination of @@ -159,44 +163,64 @@ function $InterpolateProvider() { // that's used is assigned or constructed by some JS code somewhere that is more testable or // make it obvious that you bound the value to some user controlled value. This helps reduce // the load when auditing for XSS issues. - if (trustedContext && parts.length > 1) { + if (trustedContext && hasInterpolation && (hasText || expressions.length > 1)) { throw $interpolateMinErr('noconcat', "Error while interpolating: {0}\nStrict Contextual Escaping disallows " + "interpolations that concatenate multiple expressions when a trusted value is " + "required. See http://docs.angularjs.org/api/ng.$sce", text); } - if (!mustHaveExpression || hasInterpolation) { - concat.length = length; + if (!mustHaveExpression || hasInterpolation) { + concat.length = separators.length + expressions.length; + var computeFn = function (values, context) { + for(var i = 0, ii = expressions.length; i < ii; i++) { + concat[2*i] = separators[i]; + concat[(2*i)+1] = values ? values[i] : expressions[i](context); + } + concat[2*ii] = separators[ii]; + return concat.join(''); + }; + fn = function(context) { + return computeFn(null, context); + }; + fn.exp = text; + + // hack in order to preserve existing api + fn.$$invoke = function (listener) { + return function (values, oldValues, scope) { + var current = computeFn(values, scope); + listener(current, this.$$lastInter == null ? current : this.$$lastInter, scope); + this.$$lastInter = current; + }; + }; + fn.separators = separators; + fn.expressions = expressions; + return fn; + } + + function interpolateParse(expression) { + var exp = $parse(expression); + return function (scope) { try { - for(var i = 0, ii = length, part; i Date: Thu, 6 Mar 2014 16:00:27 -0500 Subject: [PATCH 03/11] perf($compile): watch interpolated expressions individually --- src/ng/compile.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/ng/compile.js b/src/ng/compile.js index 41c45f5b0661..78479722009a 100644 --- a/src/ng/compile.js +++ b/src/ng/compile.js @@ -1804,9 +1804,9 @@ function $CompileProvider($provide, $$sanitizeUriProvider) { bindings = parent.data('$binding') || []; bindings.push(interpolateFn); safeAddClass(parent.data('$binding', bindings), 'ng-binding'); - scope.$watch(interpolateFn, function interpolateFnWatchAction(value) { + scope.$watchGroup(interpolateFn.expressions, interpolateFn.$$invoke(function(value) { node[0].nodeValue = value; - }); + })); }) }); } @@ -1867,7 +1867,7 @@ function $CompileProvider($provide, $$sanitizeUriProvider) { attr[name] = interpolateFn(scope); ($$observers[name] || ($$observers[name] = [])).$$inter = true; (attr.$$observers && attr.$$observers[name].$$scope || scope). - $watch(interpolateFn, function interpolateFnWatchAction(newValue, oldValue) { + $watchGroup(interpolateFn.expressions, interpolateFn.$$invoke(function (newValue, oldValue) { //special case for class attribute addition + removal //so that class changes can tap into the animation //hooks provided by the $animate service. Be sure to @@ -1879,7 +1879,7 @@ function $CompileProvider($provide, $$sanitizeUriProvider) { } else { attr.$set(name, newValue); } - }); + })); } }; } From d105391063034bc13108c37de598cd01e70b105c Mon Sep 17 00:00:00 2001 From: Igor Minar Date: Fri, 18 Apr 2014 15:35:37 -0700 Subject: [PATCH 04/11] WIP: working before refactoring with some extra changes --- src/ng/compile.js | 31 +++++--- src/ng/directive/ngPluralize.js | 2 +- src/ng/directive/select.js | 12 ++- src/ng/interpolate.js | 99 ++++++++++++----------- src/ngScenario/Scenario.js | 37 ++++----- test/BinderSpec.js | 24 ------ test/ng/compileSpec.js | 2 + test/ng/interpolateSpec.js | 136 ++++++++++++++------------------ test/ng/parseSpec.js | 15 +++- 9 files changed, 172 insertions(+), 186 deletions(-) diff --git a/src/ng/compile.js b/src/ng/compile.js index 78479722009a..782d0734d411 100644 --- a/src/ng/compile.js +++ b/src/ng/compile.js @@ -1804,9 +1804,10 @@ function $CompileProvider($provide, $$sanitizeUriProvider) { bindings = parent.data('$binding') || []; bindings.push(interpolateFn); safeAddClass(parent.data('$binding', bindings), 'ng-binding'); - scope.$watchGroup(interpolateFn.expressions, interpolateFn.$$invoke(function(value) { - node[0].nodeValue = value; - })); + scope.$watchGroup(interpolateFn.expressions, + function textInterpolationWatchAction(newValues) { + node[0].nodeValue = interpolateFn.compute(newValues); + }); }) }); } @@ -1847,6 +1848,8 @@ function $CompileProvider($provide, $$sanitizeUriProvider) { return { pre: function attrInterpolatePreLinkFn(scope, element, attr) { var $$observers = (attr.$$observers || (attr.$$observers = {})); + var interpolationResult; + var lastInterpolationResult; if (EVENT_HANDLER_ATTR_REGEXP.test(name)) { throw $compileMinErr('nodomevents', @@ -1862,24 +1865,30 @@ function $CompileProvider($provide, $$sanitizeUriProvider) { // register any observers if (!interpolateFn) return; - // TODO(i): this should likely be attr.$set(name, iterpolateFn(scope) so that we reset the - // actual attr value - attr[name] = interpolateFn(scope); + // initialize attr object so that it's ready in case we need the value for isolate + // scope initialization, otherwise the value would not be available from isolate + // directive's linking fn during linking phase + attr[name] = interpolationResult = interpolateFn(scope); + ($$observers[name] || ($$observers[name] = [])).$$inter = true; (attr.$$observers && attr.$$observers[name].$$scope || scope). - $watchGroup(interpolateFn.expressions, interpolateFn.$$invoke(function (newValue, oldValue) { + $watchGroup(interpolateFn.expressions, + function interpolationWatchAction(newValues) { + + lastInterpolationResult = interpolationResult; + interpolationResult = interpolateFn.compute(newValues); //special case for class attribute addition + removal //so that class changes can tap into the animation //hooks provided by the $animate service. Be sure to //skip animations when the first digest occurs (when //both the new and the old values are the same) since //the CSS classes are the non-interpolated values - if(name === 'class' && newValue != oldValue) { - attr.$updateClass(newValue, oldValue); + if(name === 'class' && interpolationResult != lastInterpolationResult) { + attr.$updateClass(interpolationResult, lastInterpolationResult); } else { - attr.$set(name, newValue); + attr.$set(name, interpolationResult); } - })); + }); } }; } diff --git a/src/ng/directive/ngPluralize.js b/src/ng/directive/ngPluralize.js index 1962d2042def..0585009a5128 100644 --- a/src/ng/directive/ngPluralize.js +++ b/src/ng/directive/ngPluralize.js @@ -204,7 +204,7 @@ var ngPluralizeDirective = ['$locale', '$interpolate', function($locale, $interp //if explicit number rule such as 1, 2, 3... is defined, just use it. Otherwise, //check it against pluralization rules in $locale service if (!(value in whens)) value = $locale.pluralCat(value - offset); - return whensExpFns[value](scope, element, true); + return whensExpFns[value](scope); } else { return ''; } diff --git a/src/ng/directive/select.js b/src/ng/directive/select.js index 628d21776b85..958e6b3dfbb2 100644 --- a/src/ng/directive/select.js +++ b/src/ng/directive/select.js @@ -609,6 +609,8 @@ var optionDirective = ['$interpolate', function($interpolate) { parent = element.parent(), selectCtrl = parent.data(selectCtrlName) || parent.parent().data(selectCtrlName); // in case we are in optgroup + var newString; + var oldString; if (selectCtrl && selectCtrl.databound) { // For some reason Opera defaults to true and if not overridden this messes up the repeater. @@ -619,10 +621,12 @@ var optionDirective = ['$interpolate', function($interpolate) { } if (interpolateFn) { - scope.$watch(interpolateFn, function interpolateWatchAction(newVal, oldVal) { - attr.$set('value', newVal); - if (newVal !== oldVal) selectCtrl.removeOption(oldVal); - selectCtrl.addOption(newVal); + scope.$watchGroup(interpolateFn.expressions, function interpolateWatchAction(newVals, oldVals) { + oldString = newString; + newString = interpolateFn.compute(newVals); + attr.$set('value', newString); + if (oldString) selectCtrl.removeOption(oldString); + selectCtrl.addOption(newString); }); } else { selectCtrl.addOption(attr.value); diff --git a/src/ng/interpolate.js b/src/ng/interpolate.js index e889133eb018..87ad2de7303a 100644 --- a/src/ng/interpolate.js +++ b/src/ng/interpolate.js @@ -114,12 +114,15 @@ function $InterpolateProvider() { * result through {@link ng.$sce#getTrusted $sce.getTrusted(interpolatedResult, * trustedContext)} before returning it. Refer to the {@link ng.$sce $sce} service that * provides Strict Contextual Escaping for details. - * @returns {function(context)} an interpolation function which is used to compute the - * interpolated string. The function has these parameters: + * @returns {Object} An object describing the interpolation template string. * - * * `context`: an object against which any expressions embedded in the strings are evaluated - * against. + * The properties of the returned object include: * + * - `template` — `{string}` — original interpolation template string. + * - `separators` — `{Array.}` — array of separators extracted from the template. + * - `expressions` — `{Array.}` — array of expressions extracted from the template. + * - `compute` — {function(Array)()} — function that when called with an array of values will + * compute the result of interpolation for the given interpolation template and values. */ function $interpolate(text, mustHaveExpression, trustedContext) { var startIndex, @@ -139,8 +142,8 @@ function $InterpolateProvider() { ((endIndex = text.indexOf(endSymbol, startIndex + startSymbolLength)) != -1) ) { if (index !== startIndex) hasText = true; separators.push(text.substring(index, startIndex)); - expressions.push(fn = interpolateParse(exp = text.substring(startIndex + startSymbolLength, endIndex))); - fn.exp = exp; + exp = text.substring(startIndex + startSymbolLength, endIndex); + expressions.push(exp); index = endIndex + endSymbolLength; hasInterpolation = true; } else { @@ -172,55 +175,51 @@ function $InterpolateProvider() { if (!mustHaveExpression || hasInterpolation) { concat.length = separators.length + expressions.length; - var computeFn = function (values, context) { - for(var i = 0, ii = expressions.length; i < ii; i++) { - concat[2*i] = separators[i]; - concat[(2*i)+1] = values ? values[i] : expressions[i](context); + + return extend(function interpolationFn(scope) { + var values = []; + forEach(interpolationFn.expressions, function(expression) { + values.push(scope.$eval(expression)); + }); + return interpolationFn.compute(values); + }, { + exp: text, //deprecated + template: text, + separators: separators, + expressions: expressions, + compute: function(values) { + for(var i = 0, ii = expressions.length; i < ii; i++) { + concat[2*i] = separators[i]; + concat[(2*i)+1] = stringify(values[i]); + } + concat[2*ii] = separators[ii]; + return concat.join(''); } - concat[2*ii] = separators[ii]; - return concat.join(''); - }; + }); + } - fn = function(context) { - return computeFn(null, context); - }; - fn.exp = text; + function stringify(value) { + try { - // hack in order to preserve existing api - fn.$$invoke = function (listener) { - return function (values, oldValues, scope) { - var current = computeFn(values, scope); - listener(current, this.$$lastInter == null ? current : this.$$lastInter, scope); - this.$$lastInter = current; - }; - }; - fn.separators = separators; - fn.expressions = expressions; - return fn; - } + if (trustedContext) { + value = $sce.getTrusted(trustedContext, value); + } else { + value = $sce.valueOf(value); + } - function interpolateParse(expression) { - var exp = $parse(expression); - return function (scope) { - try { - var value = exp(scope); - if (trustedContext) { - value = $sce.getTrusted(trustedContext, value); - } else { - value = $sce.valueOf(value); - } - if (value === null || isUndefined(value)) { - value = ''; - } else if (typeof value != 'string') { - value = toJson(value); - } - return value; - } catch(err) { - var newErr = $interpolateMinErr('interr', "Can't interpolate: {0}\n{1}", text, - err.toString()); - $exceptionHandler(newErr); + if (value === null || isUndefined(value)) { + value = ''; + } else if (typeof value != 'string') { + value = toJson(value); } - }; + + return value; + + } catch(err) { + var newErr = $interpolateMinErr('interr', "Can't interpolate: {0}\n{1}", text, + err.toString()); + $exceptionHandler(newErr); + } } } diff --git a/src/ngScenario/Scenario.js b/src/ngScenario/Scenario.js index a8e6e9d1f591..bafcba059455 100644 --- a/src/ngScenario/Scenario.js +++ b/src/ngScenario/Scenario.js @@ -302,27 +302,24 @@ _jQuery.fn.bindings = function(windowJquery, bindExp) { selection.each(function() { var element = windowJquery(this), - binding; - if (binding = element.data('$binding')) { - if (typeof binding == 'string') { - if (match(binding)) { - push(element.scope().$eval(binding)); - } - } else { - if (!angular.isArray(binding)) { - binding = [binding]; + bindings; + if (bindings = element.data('$binding')) { + if (!angular.isArray(bindings)) { + bindings = [bindings]; + } + for(var expressions = [], binding, j=0, jj=bindings.length; j{{error.throw()}}', null, true)($rootScope); - var errorLogs = $exceptionHandler.errors; - - $rootScope.error = { - 'throw': function() {throw 'ErrorMsg1';} - }; - $rootScope.$apply(); - - $rootScope.error['throw'] = function() {throw 'MyError';}; - errorLogs.length = 0; - $rootScope.$apply(); - expect(errorLogs.shift().message).toMatch(/^\[\$interpolate:interr\] Can't interpolate: \{\{error.throw\(\)\}\}\nMyError/); - - $rootScope.error['throw'] = function() {return 'ok';}; - $rootScope.$apply(); - expect(errorLogs.length).toBe(0); - }); - }); - it('IfAttrBindingThrowsErrorDecorateTheAttribute', function() { module(function($exceptionHandlerProvider){ $exceptionHandlerProvider.mode('log'); diff --git a/test/ng/compileSpec.js b/test/ng/compileSpec.js index 2b6a100fdb44..4e6d85c7dc1d 100755 --- a/test/ng/compileSpec.js +++ b/test/ng/compileSpec.js @@ -2080,6 +2080,8 @@ describe('$compile', function() { it('should set interpolated attrs to initial interpolation value', inject(function($rootScope, $compile) { + // we need the interpolated attributes to be initialized so that linking fn in a component + // can access the value during link $rootScope.whatever = 'test value'; $compile('
')($rootScope); expect(directiveAttrs.someAttr).toBe($rootScope.whatever); diff --git a/test/ng/interpolateSpec.js b/test/ng/interpolateSpec.js index ccc824aefec6..1fff54916e9d 100644 --- a/test/ng/interpolateSpec.js +++ b/test/ng/interpolateSpec.js @@ -2,9 +2,17 @@ describe('$interpolate', function() { - it('should return a function when there are no bindings and textOnly is undefined', - inject(function($interpolate) { - expect(typeof $interpolate('some text')).toBe('function'); + it('should return the interpolation object when there are no bindings and textOnly is undefined', + inject(function($interpolate, $rootScope) { + var interpolateFn = $interpolate('some text'); + + expect(interpolateFn.exp).toBe('some text'); + expect(interpolateFn.template).toBe('some text'); + expect(interpolateFn.separators).toEqual(['some text']); + expect(interpolateFn.expressions).toEqual([]); + + expect(interpolateFn({})).toBe('some text'); + expect(interpolateFn.compute([])).toBe('some text'); })); @@ -14,63 +22,43 @@ describe('$interpolate', function() { })); it('should suppress falsy objects', inject(function($interpolate) { - expect($interpolate('{{undefined}}')()).toEqual(''); - expect($interpolate('{{undefined+undefined}}')()).toEqual(''); - expect($interpolate('{{null}}')()).toEqual(''); - expect($interpolate('{{a.b}}')()).toEqual(''); + expect($interpolate('{{undefined}}').compute([undefined])).toEqual(''); + expect($interpolate('{{null}}').compute([null])).toEqual(''); + expect($interpolate('{{a.b}}').compute([undefined])).toEqual(''); })); it('should jsonify objects', inject(function($interpolate) { - expect($interpolate('{{ {} }}')()).toEqual('{}'); - expect($interpolate('{{ true }}')()).toEqual('true'); - expect($interpolate('{{ false }}')()).toEqual('false'); - })); - - it('should rethrow exceptions', inject(function($interpolate, $rootScope) { - $rootScope.err = function () { - throw new Error('oops'); - }; - expect(function () { - $interpolate('{{err()}}')($rootScope); - }).toThrowMinErr("$interpolate", "interr", "Can't interpolate: {{err()}}\nError: oops"); - })); - - it('should stop interpolation when encountering an exception', inject(function($interpolate, $compile, $rootScope) { - $rootScope.err = function () { - throw new Error('oops'); - }; - var dom = jqLite('
{{1 + 1}}
{{err()}}
{{1 + 2}}
'); - $compile(dom)($rootScope); - expect(function () { - $rootScope.$apply(); - }).toThrowMinErr("$interpolate", "interr", "Can't interpolate: {{err()}}\nError: oops"); - expect(dom[0].innerHTML).toEqual('2'); - expect(dom[1].innerHTML).toEqual('{{err()}}'); - expect(dom[2].innerHTML).toEqual('{{1 + 2}}'); + expect($interpolate('{{ {} }}').compute([{}])).toEqual('{}'); + expect($interpolate('{{ true }}').compute([true])).toEqual('true'); + expect($interpolate('{{ false }}').compute([false])).toEqual('false'); })); it('should return interpolation function', inject(function($interpolate, $rootScope) { - $rootScope.name = 'Misko'; - expect($interpolate('Hello {{name}}!')($rootScope)).toEqual('Hello Misko!'); - })); + var interpolateFn = $interpolate('Hello {{name}}!'); + expect(interpolateFn.exp).toBe('Hello {{name}}!'); + expect(interpolateFn.template).toBe('Hello {{name}}!'); + expect(interpolateFn.separators).toEqual(['Hello ', '!']); + expect(interpolateFn.expressions).toEqual(['name']); - it('should ignore undefined model', inject(function($interpolate) { - expect($interpolate("Hello {{'World' + foo}}")()).toEqual('Hello World'); + var scope = $rootScope.$new(); + scope.name = 'Bubu'; + + expect(interpolateFn(scope)).toBe('Hello Bubu!'); + expect(interpolateFn.compute(['Bubu'])).toBe('Hello Bubu!'); })); - it('should ignore undefined return value', inject(function($interpolate, $rootScope) { - $rootScope.foo = function() {return undefined}; - expect($interpolate("Hello {{'World' + foo()}}")($rootScope)).toEqual('Hello World'); + it('should ignore undefined model', inject(function($interpolate) { + expect($interpolate("Hello {{'World'}}{{foo}}").compute(['World'])).toBe('Hello World'); })); describe('interpolating in a trusted context', function() { var sce; beforeEach(function() { - function log() {}; + function log() {} var fakeLog = {log: log, warn: log, info: log, error: log}; module(function($provide, $sceProvider) { $provide.value('$log', fakeLog); @@ -81,22 +69,26 @@ describe('$interpolate', function() { it('should NOT interpolate non-trusted expressions', inject(function($interpolate) { var foo = "foo"; - expect($interpolate('{{foo}}', true, sce.CSS)({}, {foo: foo})).toEqual(''); + expect(function() { + $interpolate('{{foo}}', true, sce.CSS).compute([foo]); + }).toThrowMinErr('$interpolate', 'interr'); })); it('should NOT interpolate mistyped expressions', inject(function($interpolate) { var foo = sce.trustAsCss("foo"); - expect($interpolate('{{foo}}', true, sce.HTML)({}, {foo: foo})).toEqual(''); + expect(function() { + $interpolate('{{foo}}', true, sce.HTML).compute([foo]); + }).toThrowMinErr('$interpolate', 'interr'); })); it('should interpolate trusted expressions in a regular context', inject(function($interpolate) { var foo = sce.trustAsCss("foo"); - expect($interpolate('{{foo}}', true)({foo: foo})).toEqual('foo'); + expect($interpolate('{{foo}}', true).compute([foo])).toEqual('foo'); })); it('should interpolate trusted expressions in a specific trustedContext', inject(function($interpolate) { var foo = sce.trustAsCss("foo"); - expect($interpolate('{{foo}}', true, sce.CSS)({foo: foo})).toEqual('foo'); + expect($interpolate('{{foo}}', true, sce.CSS).compute([foo])).toEqual('foo'); })); // The concatenation of trusted values does not necessarily result in a trusted value. (For @@ -106,8 +98,8 @@ describe('$interpolate', function() { var foo = sce.trustAsCss("foo"); var bar = sce.trustAsCss("bar"); expect(function() { - return $interpolate('{{foo}}{{bar}}', true, sce.CSS)( - {foo: foo, bar: bar}); }).toThrowMinErr( + return $interpolate('{{foo}}{{bar}}', true, sce.CSS).compute([foo, bar]); + }).toThrowMinErr( "$interpolate", "noconcat", "Error while interpolating: {{foo}}{{bar}}\n" + "Strict Contextual Escaping disallows interpolations that concatenate multiple " + "expressions when a trusted value is required. See " + @@ -125,8 +117,8 @@ describe('$interpolate', function() { it('should not get confused with same markers', inject(function($interpolate) { expect($interpolate('---').separators).toEqual(['---']); expect($interpolate('---').expressions).toEqual([]); - expect($interpolate('----')()).toEqual(''); - expect($interpolate('--1--')()).toEqual('1'); + expect($interpolate('----').compute([])).toEqual(''); + expect($interpolate('--1--').compute([1])).toEqual('1'); })); }); @@ -145,62 +137,56 @@ describe('$interpolate', function() { var interpolateFn = $interpolate("a{{b}}C"), separators = interpolateFn.separators, expressions = interpolateFn.expressions; expect(separators).toEqual(['a', 'C']); - expect(expressions.length).toEqual(1); - expect(expressions[0].exp).toEqual('b'); - expect(expressions[0]({b:123})).toEqual('123'); + expect(expressions).toEqual(['b']); + expect(interpolateFn.compute([123])).toEqual('a123C'); })); it('should Parse Ending Binding', inject(function($interpolate) { var interpolateFn = $interpolate("a{{b}}"), separators = interpolateFn.separators, expressions = interpolateFn.expressions; expect(separators).toEqual(['a', '']); - expect(expressions.length).toEqual(1); - expect(expressions[0].exp).toEqual('b'); - expect(expressions[0]({b:123})).toEqual('123'); + expect(expressions).toEqual(['b']); + expect(interpolateFn.compute([123])).toEqual('a123'); })); it('should Parse Begging Binding', inject(function($interpolate) { var interpolateFn = $interpolate("{{b}}c"), separators = interpolateFn.separators, expressions = interpolateFn.expressions; expect(separators).toEqual(['', 'c']); - expect(expressions.length).toEqual(1); - expect(expressions[0].exp).toEqual('b'); - expect(expressions[0]({b:123})).toEqual('123'); + expect(expressions).toEqual(['b']); + expect(interpolateFn.compute([123])).toEqual('123c'); })); it('should Parse Loan Binding', inject(function($interpolate) { var interpolateFn = $interpolate("{{b}}"), separators = interpolateFn.separators, expressions = interpolateFn.expressions; expect(separators).toEqual(['', '']); - expect(expressions.length).toEqual(1); - expect(expressions[0].exp).toEqual('b'); - expect(expressions[0]({b:123})).toEqual('123'); + expect(expressions).toEqual(['b']); + expect(interpolateFn.compute([123])).toEqual('123'); })); it('should Parse Two Bindings', inject(function($interpolate) { var interpolateFn = $interpolate("{{b}}{{c}}"), separators = interpolateFn.separators, expressions = interpolateFn.expressions; expect(separators).toEqual(['', '', '']); - expect(expressions.length).toEqual(2); - expect(expressions[0].exp).toEqual('b'); - expect(expressions[1].exp).toEqual('c'); + expect(expressions).toEqual(['b', 'c']); + expect(interpolateFn.compute([111, 222])).toEqual('111222'); })); it('should Parse Two Bindings With Text In Middle', inject(function($interpolate) { var interpolateFn = $interpolate("{{b}}x{{c}}"), separators = interpolateFn.separators, expressions = interpolateFn.expressions; expect(separators).toEqual(['', 'x', '']); - expect(expressions.length).toEqual(2); - expect(expressions[0].exp).toEqual('b'); - expect(expressions[1].exp).toEqual('c'); + expect(expressions).toEqual(['b', 'c']); + expect(interpolateFn.compute([111, 222])).toEqual('111x222'); })); it('should Parse Multiline', inject(function($interpolate) { var interpolateFn = $interpolate('"X\nY{{A\n+B}}C\nD"'), separators = interpolateFn.separators, expressions = interpolateFn.expressions; expect(separators).toEqual(['"X\nY', 'C\nD"']); - expect(expressions.length).toEqual(1); - expect(expressions[0].exp).toEqual('A\n+B'); + expect(expressions).toEqual(['A\n+B']); + expect(interpolateFn.compute([123])).toEqual('"X\nY123C\nD"'); })); }); @@ -229,9 +215,9 @@ describe('$interpolate', function() { })); it('should interpolate a multi-part expression when isTrustedContext is false', inject(function($interpolate) { - expect($interpolate('some/{{id}}')()).toEqual('some/'); - expect($interpolate('some/{{id}}')({id: 1})).toEqual('some/1'); - expect($interpolate('{{foo}}{{bar}}')({foo: 1, bar: 2})).toEqual('12'); + expect($interpolate('some/{{id}}').compute([undefined])).toEqual('some/'); + expect($interpolate('some/{{id}}').compute([1])).toEqual('some/1'); + expect($interpolate('{{foo}}{{bar}}').compute([1, 2])).toEqual('12'); })); }); @@ -263,8 +249,8 @@ describe('$interpolate', function() { inject(function($interpolate) { expect($interpolate('---').separators).toEqual(['---']); expect($interpolate('---').expressions).toEqual([]); - expect($interpolate('----')()).toEqual(''); - expect($interpolate('--1--')()).toEqual('1'); + expect($interpolate('----').compute([])).toEqual(''); + expect($interpolate('--1--').compute([1])).toEqual('1'); }); }); }); diff --git a/test/ng/parseSpec.js b/test/ng/parseSpec.js index a76fa86b6de4..cf6d639bb480 100644 --- a/test/ng/parseSpec.js +++ b/test/ng/parseSpec.js @@ -1045,7 +1045,7 @@ describe('parser', function() { })); }); - describe('nulls in expressions', function() { + describe('null/undefined in expressions', function() { // simpleGetterFn1 it('should return null for `a` where `a` is null', inject(function($rootScope) { $rootScope.a = null; @@ -1092,6 +1092,19 @@ describe('parser', function() { $rootScope.a = { b: { c: { d: { e: { f: null } } } } }; expect($rootScope.$eval('a.b.c.d.e.f.g')).toBeUndefined(); })); + + + it('should return undefined if the return value of a function invocation is undefined', + inject(function($rootScope) { + $rootScope.fn = function() {}; + expect($rootScope.$eval('fn()')).toBeUndefined(); + })); + + it('should ignore undefined values when doing addition/concatenation', + inject(function($rootScope) { + $rootScope.fn = function() {}; + expect($rootScope.$eval('foo + "bar" + fn()')).toBe('bar'); + })); }); }); }); From eeb3d411caf638a9310606bf9ba6d98df68103db Mon Sep 17 00:00:00 2001 From: Igor Minar Date: Fri, 18 Apr 2014 16:20:02 -0700 Subject: [PATCH 05/11] WIP: speed up the returned interpolation fn using the same trick as watchGroup --- src/ng/interpolate.js | 99 +++++++++++++++++++++----------------- test/ng/interpolateSpec.js | 18 ++++--- 2 files changed, 66 insertions(+), 51 deletions(-) diff --git a/src/ng/interpolate.js b/src/ng/interpolate.js index 87ad2de7303a..46423e8fa70f 100644 --- a/src/ng/interpolate.js +++ b/src/ng/interpolate.js @@ -114,15 +114,11 @@ function $InterpolateProvider() { * result through {@link ng.$sce#getTrusted $sce.getTrusted(interpolatedResult, * trustedContext)} before returning it. Refer to the {@link ng.$sce $sce} service that * provides Strict Contextual Escaping for details. - * @returns {Object} An object describing the interpolation template string. + * @returns {function(scope)} an interpolation function which is used to compute the + * interpolated string. The function has these parameters: * - * The properties of the returned object include: - * - * - `template` — `{string}` — original interpolation template string. - * - `separators` — `{Array.}` — array of separators extracted from the template. - * - `expressions` — `{Array.}` — array of expressions extracted from the template. - * - `compute` — {function(Array)()} — function that when called with an array of values will - * compute the result of interpolation for the given interpolation template and values. + * - `scope`: a Scope instance against which any expressions embedded in the strings are + * evaluated. */ function $interpolate(text, mustHaveExpression, trustedContext) { var startIndex, @@ -133,7 +129,6 @@ function $InterpolateProvider() { textLength = text.length, hasInterpolation = false, hasText = false, - fn, exp, concat = []; @@ -176,50 +171,68 @@ function $InterpolateProvider() { if (!mustHaveExpression || hasInterpolation) { concat.length = separators.length + expressions.length; + var lastValues = []; + var lastResult; + var compute = function(values) { + for(var i = 0, ii = expressions.length; i < ii; i++) { + concat[2*i] = separators[i]; + concat[(2*i)+1] = stringify(values[i]); + } + concat[2*ii] = separators[ii]; + return concat.join(''); + }; + return extend(function interpolationFn(scope) { - var values = []; - forEach(interpolationFn.expressions, function(expression) { - values.push(scope.$eval(expression)); - }); - return interpolationFn.compute(values); + var i = 0; + var ii = expressions.length; + var values = new Array(ii); + var val; + var inputsChanged = lastResult === undefined ? true: false; + + try { + for (; i < ii; i++) { + val = scope.$eval(expressions[i]); + if (val !== lastValues[i]) { + inputsChanged = true; + } + values[i] = val; + } + + if (inputsChanged) { + lastValues = values; + lastResult = compute(values); + } + } catch(err) { + var newErr = $interpolateMinErr('interr', "Can't interpolate: {0}\n{1}", text, + err.toString()); + $exceptionHandler(newErr); + } + + return lastResult; }, { - exp: text, //deprecated - template: text, + // all of these properties are undocumented for now + exp: text, //just for compatibility with regular watchers created via $watch separators: separators, expressions: expressions, - compute: function(values) { - for(var i = 0, ii = expressions.length; i < ii; i++) { - concat[2*i] = separators[i]; - concat[(2*i)+1] = stringify(values[i]); - } - concat[2*ii] = separators[ii]; - return concat.join(''); - } + // TODO(i): remove. don't expose this at all. requires changing many tests + compute: compute }); } function stringify(value) { - try { - - if (trustedContext) { - value = $sce.getTrusted(trustedContext, value); - } else { - value = $sce.valueOf(value); - } - - if (value === null || isUndefined(value)) { - value = ''; - } else if (typeof value != 'string') { - value = toJson(value); - } - - return value; + if (trustedContext) { + value = $sce.getTrusted(trustedContext, value); + } else { + value = $sce.valueOf(value); + } - } catch(err) { - var newErr = $interpolateMinErr('interr', "Can't interpolate: {0}\n{1}", text, - err.toString()); - $exceptionHandler(newErr); + if (value === null || isUndefined(value)) { + value = ''; + } else if (typeof value != 'string') { + value = toJson(value); } + + return value; } } diff --git a/test/ng/interpolateSpec.js b/test/ng/interpolateSpec.js index 1fff54916e9d..8ad42999bcc2 100644 --- a/test/ng/interpolateSpec.js +++ b/test/ng/interpolateSpec.js @@ -7,7 +7,6 @@ describe('$interpolate', function() { var interpolateFn = $interpolate('some text'); expect(interpolateFn.exp).toBe('some text'); - expect(interpolateFn.template).toBe('some text'); expect(interpolateFn.separators).toEqual(['some text']); expect(interpolateFn.expressions).toEqual([]); @@ -38,7 +37,6 @@ describe('$interpolate', function() { var interpolateFn = $interpolate('Hello {{name}}!'); expect(interpolateFn.exp).toBe('Hello {{name}}!'); - expect(interpolateFn.template).toBe('Hello {{name}}!'); expect(interpolateFn.separators).toEqual(['Hello ', '!']); expect(interpolateFn.expressions).toEqual(['name']); @@ -67,17 +65,21 @@ describe('$interpolate', function() { inject(['$sce', function($sce) { sce = $sce; }]); }); - it('should NOT interpolate non-trusted expressions', inject(function($interpolate) { - var foo = "foo"; + it('should NOT interpolate non-trusted expressions', inject(function($interpolate, $rootScope) { + var scope = $rootScope.$new(); + scope.foo = "foo"; + expect(function() { - $interpolate('{{foo}}', true, sce.CSS).compute([foo]); + $interpolate('{{foo}}', true, sce.CSS)(scope); }).toThrowMinErr('$interpolate', 'interr'); })); - it('should NOT interpolate mistyped expressions', inject(function($interpolate) { - var foo = sce.trustAsCss("foo"); + it('should NOT interpolate mistyped expressions', inject(function($interpolate, $rootScope) { + var scope = $rootScope.$new(); + scope.foo = sce.trustAsCss("foo"); + expect(function() { - $interpolate('{{foo}}', true, sce.HTML).compute([foo]); + $interpolate('{{foo}}', true, sce.HTML)(scope); }).toThrowMinErr('$interpolate', 'interr'); })); From e5321a4ebb4cb03dab9140efe6fbd545579bb2e0 Mon Sep 17 00:00:00 2001 From: Igor Minar Date: Fri, 18 Apr 2014 16:01:16 -0700 Subject: [PATCH 06/11] WIP: undo watchgroup use --- src/ng/compile.js | 21 +++++++-------------- src/ng/directive/select.js | 14 ++++++-------- 2 files changed, 13 insertions(+), 22 deletions(-) diff --git a/src/ng/compile.js b/src/ng/compile.js index 782d0734d411..4256adcaabc7 100644 --- a/src/ng/compile.js +++ b/src/ng/compile.js @@ -1804,9 +1804,8 @@ function $CompileProvider($provide, $$sanitizeUriProvider) { bindings = parent.data('$binding') || []; bindings.push(interpolateFn); safeAddClass(parent.data('$binding', bindings), 'ng-binding'); - scope.$watchGroup(interpolateFn.expressions, - function textInterpolationWatchAction(newValues) { - node[0].nodeValue = interpolateFn.compute(newValues); + scope.$watch(interpolateFn, function interpolateFnWatchAction(value) { + node[0].nodeValue = value; }); }) }); @@ -1848,8 +1847,6 @@ function $CompileProvider($provide, $$sanitizeUriProvider) { return { pre: function attrInterpolatePreLinkFn(scope, element, attr) { var $$observers = (attr.$$observers || (attr.$$observers = {})); - var interpolationResult; - var lastInterpolationResult; if (EVENT_HANDLER_ATTR_REGEXP.test(name)) { throw $compileMinErr('nodomevents', @@ -1868,25 +1865,21 @@ function $CompileProvider($provide, $$sanitizeUriProvider) { // initialize attr object so that it's ready in case we need the value for isolate // scope initialization, otherwise the value would not be available from isolate // directive's linking fn during linking phase - attr[name] = interpolationResult = interpolateFn(scope); + attr[name] = interpolateFn(scope); ($$observers[name] || ($$observers[name] = [])).$$inter = true; (attr.$$observers && attr.$$observers[name].$$scope || scope). - $watchGroup(interpolateFn.expressions, - function interpolationWatchAction(newValues) { - - lastInterpolationResult = interpolationResult; - interpolationResult = interpolateFn.compute(newValues); + $watch(interpolateFn, function interpolateFnWatchAction(newValue, oldValue) { //special case for class attribute addition + removal //so that class changes can tap into the animation //hooks provided by the $animate service. Be sure to //skip animations when the first digest occurs (when //both the new and the old values are the same) since //the CSS classes are the non-interpolated values - if(name === 'class' && interpolationResult != lastInterpolationResult) { - attr.$updateClass(interpolationResult, lastInterpolationResult); + if(name === 'class' && newValue != oldValue) { + attr.$updateClass(newValue, oldValue); } else { - attr.$set(name, interpolationResult); + attr.$set(name, newValue); } }); } diff --git a/src/ng/directive/select.js b/src/ng/directive/select.js index 958e6b3dfbb2..95fe5ad7d088 100644 --- a/src/ng/directive/select.js +++ b/src/ng/directive/select.js @@ -609,8 +609,6 @@ var optionDirective = ['$interpolate', function($interpolate) { parent = element.parent(), selectCtrl = parent.data(selectCtrlName) || parent.parent().data(selectCtrlName); // in case we are in optgroup - var newString; - var oldString; if (selectCtrl && selectCtrl.databound) { // For some reason Opera defaults to true and if not overridden this messes up the repeater. @@ -621,12 +619,12 @@ var optionDirective = ['$interpolate', function($interpolate) { } if (interpolateFn) { - scope.$watchGroup(interpolateFn.expressions, function interpolateWatchAction(newVals, oldVals) { - oldString = newString; - newString = interpolateFn.compute(newVals); - attr.$set('value', newString); - if (oldString) selectCtrl.removeOption(oldString); - selectCtrl.addOption(newString); + scope.$watch(interpolateFn, function interpolateWatchAction(newVal, oldVal) { + attr.$set('value', newVal); + if (oldVal !== newVal) { + selectCtrl.removeOption(oldVal); + } + selectCtrl.addOption(newVal); }); } else { selectCtrl.addOption(attr.value); From fa254675935eeab77a991ae058b751a5086a82aa Mon Sep 17 00:00:00 2001 From: Igor Minar Date: Fri, 18 Apr 2014 17:12:00 -0700 Subject: [PATCH 07/11] WIP: remove extra cruft from $watchGroup + update tests --- src/ng/rootScope.js | 24 ++++++++---------------- test/ng/rootScopeSpec.js | 4 ++-- 2 files changed, 10 insertions(+), 18 deletions(-) diff --git a/src/ng/rootScope.js b/src/ng/rootScope.js index a9ed64620480..274b36e83b59 100644 --- a/src/ng/rootScope.js +++ b/src/ng/rootScope.js @@ -381,30 +381,22 @@ function $RootScopeProvider(){ * @returns {function()} Returns a de-registration function for all listeners. */ $watchGroup: function(watchExpressions, listener) { - var oldValues = new Array(watchExpressions.length), - newValues = new Array(watchExpressions.length); - - if (watchExpressions.length === 1) { - // Special case size of one - return this.$watch(watchExpressions[0], function (value, oldValue, scope) { - newValues[0] = value; - oldValues[0] = oldValue; - listener.call(this, newValues, oldValues, scope); - }); - } - var deregisterFns = [], - changeCount = 0; + var oldValues = new Array(watchExpressions.length); + var newValues = new Array(watchExpressions.length); + var deregisterFns = []; + var changeCount = 0; + var self = this; forEach(watchExpressions, function (expr, i) { - deregisterFns.push(this.$watch(expr, function (value, oldValue) { + deregisterFns.push(self.$watch(expr, function (value, oldValue) { newValues[i] = value; oldValues[i] = oldValue; changeCount++; })); }, this); - deregisterFns.push(this.$watch(function () {return changeCount;}, function (c, o, scope) { - listener.call(this, newValues, oldValues, scope); + deregisterFns.push(self.$watch(function () {return changeCount;}, function () { + listener(newValues, oldValues, self); })); return function deregisterWatchGroup() { diff --git a/test/ng/rootScopeSpec.js b/test/ng/rootScopeSpec.js index 77411e596eb3..92498ab6d89b 100644 --- a/test/ng/rootScopeSpec.js +++ b/test/ng/rootScopeSpec.js @@ -784,7 +784,7 @@ describe('Scope', function() { })); - it('should treat set of 1 as direct watch', function() { + it('should work for a group with just a single expression', function() { var lastValues = ['foo']; var log = ''; var clean = scope.$watchGroup(['a'], function(values, oldValues, s) { @@ -812,7 +812,7 @@ describe('Scope', function() { }); - it('should detect a change to any one in a set', function() { + it('should detect a change to any one expression in the group', function() { var lastValues = ['foo', 'bar']; var log = ''; From dcaea5f5036c203b54111a034bc569b535da29a0 Mon Sep 17 00:00:00 2001 From: Igor Minar Date: Mon, 21 Apr 2014 09:07:58 -0700 Subject: [PATCH 08/11] WIP: remove compute, use $parse, remove scope.$eval --- src/ng/interpolate.js | 15 +++++------ test/ng/interpolateSpec.js | 52 ++++++++++++++++++-------------------- 2 files changed, 32 insertions(+), 35 deletions(-) diff --git a/src/ng/interpolate.js b/src/ng/interpolate.js index 46423e8fa70f..fe2136f24047 100644 --- a/src/ng/interpolate.js +++ b/src/ng/interpolate.js @@ -114,11 +114,10 @@ function $InterpolateProvider() { * result through {@link ng.$sce#getTrusted $sce.getTrusted(interpolatedResult, * trustedContext)} before returning it. Refer to the {@link ng.$sce $sce} service that * provides Strict Contextual Escaping for details. - * @returns {function(scope)} an interpolation function which is used to compute the + * @returns {function(context)} an interpolation function which is used to compute the * interpolated string. The function has these parameters: * - * - `scope`: a Scope instance against which any expressions embedded in the strings are - * evaluated. + * - `context`: evaluation context for all expressions embedded in the interpolated text */ function $interpolate(text, mustHaveExpression, trustedContext) { var startIndex, @@ -126,6 +125,7 @@ function $InterpolateProvider() { index = 0, separators = [], expressions = [], + parseFns = [], textLength = text.length, hasInterpolation = false, hasText = false, @@ -139,6 +139,7 @@ function $InterpolateProvider() { separators.push(text.substring(index, startIndex)); exp = text.substring(startIndex + startSymbolLength, endIndex); expressions.push(exp); + parseFns.push($parse(exp)); index = endIndex + endSymbolLength; hasInterpolation = true; } else { @@ -182,7 +183,7 @@ function $InterpolateProvider() { return concat.join(''); }; - return extend(function interpolationFn(scope) { + return extend(function interpolationFn(context) { var i = 0; var ii = expressions.length; var values = new Array(ii); @@ -191,7 +192,7 @@ function $InterpolateProvider() { try { for (; i < ii; i++) { - val = scope.$eval(expressions[i]); + val = parseFns[i](context); if (val !== lastValues[i]) { inputsChanged = true; } @@ -213,9 +214,7 @@ function $InterpolateProvider() { // all of these properties are undocumented for now exp: text, //just for compatibility with regular watchers created via $watch separators: separators, - expressions: expressions, - // TODO(i): remove. don't expose this at all. requires changing many tests - compute: compute + expressions: expressions }); } diff --git a/test/ng/interpolateSpec.js b/test/ng/interpolateSpec.js index 8ad42999bcc2..864b4b87d06c 100644 --- a/test/ng/interpolateSpec.js +++ b/test/ng/interpolateSpec.js @@ -3,7 +3,7 @@ describe('$interpolate', function() { it('should return the interpolation object when there are no bindings and textOnly is undefined', - inject(function($interpolate, $rootScope) { + inject(function($interpolate) { var interpolateFn = $interpolate('some text'); expect(interpolateFn.exp).toBe('some text'); @@ -11,7 +11,6 @@ describe('$interpolate', function() { expect(interpolateFn.expressions).toEqual([]); expect(interpolateFn({})).toBe('some text'); - expect(interpolateFn.compute([])).toBe('some text'); })); @@ -21,15 +20,15 @@ describe('$interpolate', function() { })); it('should suppress falsy objects', inject(function($interpolate) { - expect($interpolate('{{undefined}}').compute([undefined])).toEqual(''); - expect($interpolate('{{null}}').compute([null])).toEqual(''); - expect($interpolate('{{a.b}}').compute([undefined])).toEqual(''); + expect($interpolate('{{undefined}}')({})).toEqual(''); + expect($interpolate('{{null}}')({})).toEqual(''); + expect($interpolate('{{a.b}}')({})).toEqual(''); })); it('should jsonify objects', inject(function($interpolate) { - expect($interpolate('{{ {} }}').compute([{}])).toEqual('{}'); - expect($interpolate('{{ true }}').compute([true])).toEqual('true'); - expect($interpolate('{{ false }}').compute([false])).toEqual('false'); + expect($interpolate('{{ {} }}')({})).toEqual('{}'); + expect($interpolate('{{ true }}')({})).toEqual('true'); + expect($interpolate('{{ false }}')({})).toEqual('false'); })); @@ -44,12 +43,11 @@ describe('$interpolate', function() { scope.name = 'Bubu'; expect(interpolateFn(scope)).toBe('Hello Bubu!'); - expect(interpolateFn.compute(['Bubu'])).toBe('Hello Bubu!'); })); it('should ignore undefined model', inject(function($interpolate) { - expect($interpolate("Hello {{'World'}}{{foo}}").compute(['World'])).toBe('Hello World'); + expect($interpolate("Hello {{'World'}}{{foo}}")({})).toBe('Hello World'); })); @@ -85,12 +83,12 @@ describe('$interpolate', function() { it('should interpolate trusted expressions in a regular context', inject(function($interpolate) { var foo = sce.trustAsCss("foo"); - expect($interpolate('{{foo}}', true).compute([foo])).toEqual('foo'); + expect($interpolate('{{foo}}', true)({foo: foo})).toBe('foo'); })); it('should interpolate trusted expressions in a specific trustedContext', inject(function($interpolate) { var foo = sce.trustAsCss("foo"); - expect($interpolate('{{foo}}', true, sce.CSS).compute([foo])).toEqual('foo'); + expect($interpolate('{{foo}}', true, sce.CSS)({foo: foo})).toBe('foo'); })); // The concatenation of trusted values does not necessarily result in a trusted value. (For @@ -100,7 +98,7 @@ describe('$interpolate', function() { var foo = sce.trustAsCss("foo"); var bar = sce.trustAsCss("bar"); expect(function() { - return $interpolate('{{foo}}{{bar}}', true, sce.CSS).compute([foo, bar]); + return $interpolate('{{foo}}{{bar}}', true, sce.CSS)({foo: foo, bar: bar}); }).toThrowMinErr( "$interpolate", "noconcat", "Error while interpolating: {{foo}}{{bar}}\n" + "Strict Contextual Escaping disallows interpolations that concatenate multiple " + @@ -119,8 +117,8 @@ describe('$interpolate', function() { it('should not get confused with same markers', inject(function($interpolate) { expect($interpolate('---').separators).toEqual(['---']); expect($interpolate('---').expressions).toEqual([]); - expect($interpolate('----').compute([])).toEqual(''); - expect($interpolate('--1--').compute([1])).toEqual('1'); + expect($interpolate('----')({})).toEqual(''); + expect($interpolate('--1--')({})).toEqual('1'); })); }); @@ -140,7 +138,7 @@ describe('$interpolate', function() { separators = interpolateFn.separators, expressions = interpolateFn.expressions; expect(separators).toEqual(['a', 'C']); expect(expressions).toEqual(['b']); - expect(interpolateFn.compute([123])).toEqual('a123C'); + expect(interpolateFn({b: 123})).toEqual('a123C'); })); it('should Parse Ending Binding', inject(function($interpolate) { @@ -148,7 +146,7 @@ describe('$interpolate', function() { separators = interpolateFn.separators, expressions = interpolateFn.expressions; expect(separators).toEqual(['a', '']); expect(expressions).toEqual(['b']); - expect(interpolateFn.compute([123])).toEqual('a123'); + expect(interpolateFn({b: 123})).toEqual('a123'); })); it('should Parse Begging Binding', inject(function($interpolate) { @@ -156,7 +154,7 @@ describe('$interpolate', function() { separators = interpolateFn.separators, expressions = interpolateFn.expressions; expect(separators).toEqual(['', 'c']); expect(expressions).toEqual(['b']); - expect(interpolateFn.compute([123])).toEqual('123c'); + expect(interpolateFn({b: 123})).toEqual('123c'); })); it('should Parse Loan Binding', inject(function($interpolate) { @@ -164,7 +162,7 @@ describe('$interpolate', function() { separators = interpolateFn.separators, expressions = interpolateFn.expressions; expect(separators).toEqual(['', '']); expect(expressions).toEqual(['b']); - expect(interpolateFn.compute([123])).toEqual('123'); + expect(interpolateFn({b: 123})).toEqual('123'); })); it('should Parse Two Bindings', inject(function($interpolate) { @@ -172,7 +170,7 @@ describe('$interpolate', function() { separators = interpolateFn.separators, expressions = interpolateFn.expressions; expect(separators).toEqual(['', '', '']); expect(expressions).toEqual(['b', 'c']); - expect(interpolateFn.compute([111, 222])).toEqual('111222'); + expect(interpolateFn({b: 111, c: 222})).toEqual('111222'); })); it('should Parse Two Bindings With Text In Middle', inject(function($interpolate) { @@ -180,7 +178,7 @@ describe('$interpolate', function() { separators = interpolateFn.separators, expressions = interpolateFn.expressions; expect(separators).toEqual(['', 'x', '']); expect(expressions).toEqual(['b', 'c']); - expect(interpolateFn.compute([111, 222])).toEqual('111x222'); + expect(interpolateFn({b: 111, c: 222})).toEqual('111x222'); })); it('should Parse Multiline', inject(function($interpolate) { @@ -188,7 +186,7 @@ describe('$interpolate', function() { separators = interpolateFn.separators, expressions = interpolateFn.expressions; expect(separators).toEqual(['"X\nY', 'C\nD"']); expect(expressions).toEqual(['A\n+B']); - expect(interpolateFn.compute([123])).toEqual('"X\nY123C\nD"'); + expect(interpolateFn({'A': 'aa', 'B': 'bb'})).toEqual('"X\nYaabbC\nD"'); })); }); @@ -217,9 +215,9 @@ describe('$interpolate', function() { })); it('should interpolate a multi-part expression when isTrustedContext is false', inject(function($interpolate) { - expect($interpolate('some/{{id}}').compute([undefined])).toEqual('some/'); - expect($interpolate('some/{{id}}').compute([1])).toEqual('some/1'); - expect($interpolate('{{foo}}{{bar}}').compute([1, 2])).toEqual('12'); + expect($interpolate('some/{{id}}')({})).toEqual('some/'); + expect($interpolate('some/{{id}}')({id: 1})).toEqual('some/1'); + expect($interpolate('{{foo}}{{bar}}')({foo: 1, bar: 2})).toEqual('12'); })); }); @@ -251,8 +249,8 @@ describe('$interpolate', function() { inject(function($interpolate) { expect($interpolate('---').separators).toEqual(['---']); expect($interpolate('---').expressions).toEqual([]); - expect($interpolate('----').compute([])).toEqual(''); - expect($interpolate('--1--').compute([1])).toEqual('1'); + expect($interpolate('----')({})).toEqual(''); + expect($interpolate('--1--')({})).toEqual('1'); }); }); }); From 590e57aa75ba7658bb7730d3de56de44a9f6d70a Mon Sep 17 00:00:00 2001 From: Igor Minar Date: Mon, 21 Apr 2014 09:33:35 -0700 Subject: [PATCH 09/11] WIP: move stringify around --- src/ng/interpolate.js | 37 +++++++++++++++++++------------------ 1 file changed, 19 insertions(+), 18 deletions(-) diff --git a/src/ng/interpolate.js b/src/ng/interpolate.js index fe2136f24047..12fb7964f970 100644 --- a/src/ng/interpolate.js +++ b/src/ng/interpolate.js @@ -174,15 +174,32 @@ function $InterpolateProvider() { var lastValues = []; var lastResult; + var compute = function(values) { for(var i = 0, ii = expressions.length; i < ii; i++) { concat[2*i] = separators[i]; - concat[(2*i)+1] = stringify(values[i]); + concat[(2*i)+1] = values[i]; } concat[2*ii] = separators[ii]; return concat.join(''); }; + var stringify = function (value) { + if (trustedContext) { + value = $sce.getTrusted(trustedContext, value); + } else { + value = $sce.valueOf(value); + } + + if (value === null || isUndefined(value)) { + value = ''; + } else if (typeof value != 'string') { + value = toJson(value); + } + + return value; + }; + return extend(function interpolationFn(context) { var i = 0; var ii = expressions.length; @@ -192,7 +209,7 @@ function $InterpolateProvider() { try { for (; i < ii; i++) { - val = parseFns[i](context); + val = stringify(parseFns[i](context)); if (val !== lastValues[i]) { inputsChanged = true; } @@ -217,22 +234,6 @@ function $InterpolateProvider() { expressions: expressions }); } - - function stringify(value) { - if (trustedContext) { - value = $sce.getTrusted(trustedContext, value); - } else { - value = $sce.valueOf(value); - } - - if (value === null || isUndefined(value)) { - value = ''; - } else if (typeof value != 'string') { - value = toJson(value); - } - - return value; - } } From d1fa727778be8756842dace0e39f60d36ef8c586 Mon Sep 17 00:00:00 2001 From: Igor Minar Date: Mon, 21 Apr 2014 09:58:48 -0700 Subject: [PATCH 10/11] WIP: use log test service in watchGroup tests --- test/ng/rootScopeSpec.js | 52 +++++++++++++++------------------------- 1 file changed, 19 insertions(+), 33 deletions(-) diff --git a/test/ng/rootScopeSpec.js b/test/ng/rootScopeSpec.js index 92498ab6d89b..13efd5d9110e 100644 --- a/test/ng/rootScopeSpec.js +++ b/test/ng/rootScopeSpec.js @@ -779,83 +779,69 @@ describe('Scope', function() { describe('$watchGroup', function() { var scope; - beforeEach(inject(function($rootScope) { + var log; + + beforeEach(inject(function($rootScope, _log_) { scope = $rootScope.$new(); + log = _log_; })); it('should work for a group with just a single expression', function() { - var lastValues = ['foo']; - var log = ''; - var clean = scope.$watchGroup(['a'], function(values, oldValues, s) { - log += values.join(',') + ';'; + scope.$watchGroup(['a'], function(values, oldValues, s) { expect(s).toBe(scope); - expect(oldValues).toEqual(lastValues); - lastValues = values.slice(); + log(oldValues + ' >>> ' + values); }); scope.a = 'foo'; scope.$digest(); - expect(log).toEqual('foo;'); + expect(log).toEqual('foo >>> foo'); + log.reset(); scope.$digest(); - expect(log).toEqual('foo;'); + expect(log).toEqual(''); scope.a = 'bar'; scope.$digest(); - expect(log).toEqual('foo;bar;'); - - clean(); - scope.a = 'xxx'; - scope.$digest(); - expect(log).toEqual('foo;bar;'); + expect(log).toEqual('foo >>> bar'); }); it('should detect a change to any one expression in the group', function() { - var lastValues = ['foo', 'bar']; - var log = ''; - scope.$watchGroup(['a', 'b'], function(values, oldValues, s) { - log += values.join(',') + ';'; expect(s).toBe(scope); - expect(oldValues).toEqual(lastValues); - lastValues = values.slice(); + log(oldValues + ' >>> ' + values); }); scope.a = 'foo'; scope.b = 'bar'; scope.$digest(); - expect(log).toEqual('foo,bar;'); + expect(log).toEqual('foo,bar >>> foo,bar'); - log = ''; + log.reset(); scope.$digest(); expect(log).toEqual(''); scope.a = 'a'; scope.$digest(); - expect(log).toEqual('a,bar;'); + expect(log).toEqual('foo,bar >>> a,bar'); - log = ''; + log.reset(); scope.a = 'A'; scope.b = 'B'; scope.$digest(); - expect(log).toEqual('A,B;'); + expect(log).toEqual('a,bar >>> A,B'); }); it('should not call watch action fn when watchGroup was deregistered', function() { - var lastValues = ['foo', 'bar']; - var log = ''; - var deregister = scope.$watchGroup(['a', 'b'], function(values, oldValues, s) { - log += values.join(',') + ';'; - expect(s).toBe(scope); - expect(oldValues).toEqual(lastValues); - lastValues = values.slice(); + var deregister = scope.$watchGroup(['a', 'b'], function(values, oldValues) { + log(oldValues + ' >>> ' + values); }); deregister(); scope.a = 'xxx'; + scope.b = 'yyy'; scope.$digest(); expect(log).toEqual(''); }); From 0b6cbf7a4b6c47b90030ba2b8b40356c016ad18f Mon Sep 17 00:00:00 2001 From: Igor Minar Date: Mon, 21 Apr 2014 11:02:04 -0700 Subject: [PATCH 11/11] WIP: keep last value/result cache associated with scopes --- src/ng/interpolate.js | 28 ++++++++++++++++++++++------ 1 file changed, 22 insertions(+), 6 deletions(-) diff --git a/src/ng/interpolate.js b/src/ng/interpolate.js index 12fb7964f970..9ac68e8b7727 100644 --- a/src/ng/interpolate.js +++ b/src/ng/interpolate.js @@ -130,7 +130,8 @@ function $InterpolateProvider() { hasInterpolation = false, hasText = false, exp, - concat = []; + concat = [], + lastValuesCache = { values: {}, results: {}}; while(index < textLength) { if ( ((startIndex = text.indexOf(startSymbol, index)) != -1) && @@ -172,9 +173,6 @@ function $InterpolateProvider() { if (!mustHaveExpression || hasInterpolation) { concat.length = separators.length + expressions.length; - var lastValues = []; - var lastResult; - var compute = function(values) { for(var i = 0, ii = expressions.length; i < ii; i++) { concat[2*i] = separators[i]; @@ -201,12 +199,30 @@ function $InterpolateProvider() { }; return extend(function interpolationFn(context) { + var scopeId = context.$id || 'notAScope'; + var lastValues = lastValuesCache.values[scopeId]; + var lastResult = lastValuesCache.results[scopeId]; var i = 0; var ii = expressions.length; var values = new Array(ii); var val; var inputsChanged = lastResult === undefined ? true: false; + + // if we haven't seen this context before, initialize the cache and try to setup + // a cleanup routine that purges the cache when the scope goes away. + if (!lastValues) { + lastValues = []; + inputsChanged = true; + if (context.$on) { + context.$on('$destroy', function() { + lastValuesCache.values[scopeId] = null; + lastValuesCache.results[scopeId] = null; + }); + } + } + + try { for (; i < ii; i++) { val = stringify(parseFns[i](context)); @@ -217,8 +233,8 @@ function $InterpolateProvider() { } if (inputsChanged) { - lastValues = values; - lastResult = compute(values); + lastValuesCache.values[scopeId] = values; + lastValuesCache.results[scopeId] = lastResult = compute(values); } } catch(err) { var newErr = $interpolateMinErr('interr', "Can't interpolate: {0}\n{1}", text,