refactor($interpolate): attempt to remove hacky code due to $interpolation perf improvements

This commit is contained in:
Igor Minar
2014-04-18 15:35:37 -07:00
parent 0ebfa0d112
commit 1db3b8cfb7
9 changed files with 174 additions and 188 deletions

View File

@@ -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);
}
}));
});
}
};
}

View File

@@ -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 '';
}

View File

@@ -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);

View File

@@ -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.<string>}` — array of separators extracted from the template.
* - `expressions` — `{Array.<string>}` — 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;
// 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 {
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);
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;
} catch(err) {
var newErr = $interpolateMinErr('interr', "Can't interpolate: {0}\n{1}", text,
err.toString());
$exceptionHandler(newErr);
}
}
}

View File

@@ -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));
bindings;
if (bindings = element.data('$binding')) {
if (!angular.isArray(bindings)) {
bindings = [bindings];
}
for(var expressions = [], binding, j=0, jj=bindings.length; j<jj; j++) {
binding = bindings[j];
if (binding.expressions) {
expressions = binding.expressions;
} else {
expressions = [binding];
}
} else {
if (!angular.isArray(binding)) {
binding = [binding];
}
for(var fns, j=0, jj=binding.length; j<jj; j++) {
fns = binding[j];
if (fns.expressions) {
fns = fns.expressions;
} else {
fns = [fns];
}
for (var scope, fn, i = 0, ii = fns.length; i < ii; i++) {
if(match((fn = fns[i]).exp)) {
push(fn(scope = scope || element.scope()));
}
for (var scope, expression, i = 0, ii = expressions.length; i < ii; i++) {
expression = expressions[i];
if(match(expression)) {
scope = scope || element.scope();
push(scope.$eval(expression));
}
}
}

View File

@@ -167,30 +167,6 @@ describe('Binder', function() {
expect(element[0].childNodes.length).toEqual(1);
}));
it('IfTextBindingThrowsErrorDecorateTheSpan', function() {
module(function($exceptionHandlerProvider){
$exceptionHandlerProvider.mode('log');
});
inject(function($rootScope, $exceptionHandler, $compile) {
element = $compile('<div>{{error.throw()}}</div>', 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');

View File

@@ -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('<div some-attr="{{whatever}}" observer></div>')($rootScope);
expect(directiveAttrs.someAttr).toBe($rootScope.whatever);

View File

@@ -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('<div>{{1 + 1}}</div><div>{{err()}}</div><div>{{1 + 2}}</div>');
$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']);
var scope = $rootScope.$new();
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}}")()).toEqual('Hello World');
}));
it('should ignore undefined return value', inject(function($interpolate, $rootScope) {
$rootScope.foo = function() {return undefined};
expect($interpolate("Hello {{'World' + foo()}}")($rootScope)).toEqual('Hello World');
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');
});
});
});

View File

@@ -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');
}));
});
});
});