Skip to content

Commit 1db3b8c

Browse files
IgorMinarIgorMinar
committed
refactor($interpolate): attempt to remove hacky code due to $interpolation perf improvements
1 parent 0ebfa0d commit 1db3b8c

9 files changed

Lines changed: 172 additions & 186 deletions

File tree

‎src/ng/compile.js‎

Lines changed: 20 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1804,9 +1804,10 @@ function $CompileProvider($provide, $$sanitizeUriProvider) {
18041804
bindings = parent.data('$binding') || [];
18051805
bindings.push(interpolateFn);
18061806
safeAddClass(parent.data('$binding', bindings), 'ng-binding');
1807-
scope.$watchGroup(interpolateFn.expressions, interpolateFn.$$invoke(function(value) {
1808-
node[0].nodeValue = value;
1809-
}));
1807+
scope.$watchGroup(interpolateFn.expressions,
1808+
function textInterpolationWatchAction(newValues) {
1809+
node[0].nodeValue = interpolateFn.compute(newValues);
1810+
});
18101811
})
18111812
});
18121813
}
@@ -1847,6 +1848,8 @@ function $CompileProvider($provide, $$sanitizeUriProvider) {
18471848
return {
18481849
pre: function attrInterpolatePreLinkFn(scope, element, attr) {
18491850
var $$observers = (attr.$$observers || (attr.$$observers = {}));
1851+
var interpolationResult;
1852+
var lastInterpolationResult;
18501853

18511854
if (EVENT_HANDLER_ATTR_REGEXP.test(name)) {
18521855
throw $compileMinErr('nodomevents',
@@ -1862,24 +1865,30 @@ function $CompileProvider($provide, $$sanitizeUriProvider) {
18621865
// register any observers
18631866
if (!interpolateFn) return;
18641867

1865-
// TODO(i): this should likely be attr.$set(name, iterpolateFn(scope) so that we reset the
1866-
// actual attr value
1867-
attr[name] = interpolateFn(scope);
1868+
// initialize attr object so that it's ready in case we need the value for isolate
1869+
// scope initialization, otherwise the value would not be available from isolate
1870+
// directive's linking fn during linking phase
1871+
attr[name] = interpolationResult = interpolateFn(scope);
1872+
18681873
($$observers[name] || ($$observers[name] = [])).$$inter = true;
18691874
(attr.$$observers && attr.$$observers[name].$$scope || scope).
1870-
$watchGroup(interpolateFn.expressions, interpolateFn.$$invoke(function (newValue, oldValue) {
1875+
$watchGroup(interpolateFn.expressions,
1876+
function interpolationWatchAction(newValues) {
1877+
1878+
lastInterpolationResult = interpolationResult;
1879+
interpolationResult = interpolateFn.compute(newValues);
18711880
//special case for class attribute addition + removal
18721881
//so that class changes can tap into the animation
18731882
//hooks provided by the $animate service. Be sure to
18741883
//skip animations when the first digest occurs (when
18751884
//both the new and the old values are the same) since
18761885
//the CSS classes are the non-interpolated values
1877-
if(name === 'class' && newValue != oldValue) {
1878-
attr.$updateClass(newValue, oldValue);
1886+
if(name === 'class' && interpolationResult != lastInterpolationResult) {
1887+
attr.$updateClass(interpolationResult, lastInterpolationResult);
18791888
} else {
1880-
attr.$set(name, newValue);
1889+
attr.$set(name, interpolationResult);
18811890
}
1882-
}));
1891+
});
18831892
}
18841893
};
18851894
}

‎src/ng/directive/ngPluralize.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -204,7 +204,7 @@ var ngPluralizeDirective = ['$locale', '$interpolate', function($locale, $interp
204204
//if explicit number rule such as 1, 2, 3... is defined, just use it. Otherwise,
205205
//check it against pluralization rules in $locale service
206206
if (!(value in whens)) value = $locale.pluralCat(value - offset);
207-
return whensExpFns[value](scope, element, true);
207+
return whensExpFns[value](scope);
208208
} else {
209209
return '';
210210
}

‎src/ng/directive/select.js‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -609,6 +609,8 @@ var optionDirective = ['$interpolate', function($interpolate) {
609609
parent = element.parent(),
610610
selectCtrl = parent.data(selectCtrlName) ||
611611
parent.parent().data(selectCtrlName); // in case we are in optgroup
612+
var newString;
613+
var oldString;
612614

613615
if (selectCtrl && selectCtrl.databound) {
614616
// 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) {
619621
}
620622

621623
if (interpolateFn) {
622-
scope.$watch(interpolateFn, function interpolateWatchAction(newVal, oldVal) {
623-
attr.$set('value', newVal);
624-
if (newVal !== oldVal) selectCtrl.removeOption(oldVal);
625-
selectCtrl.addOption(newVal);
624+
scope.$watchGroup(interpolateFn.expressions, function interpolateWatchAction(newVals, oldVals) {
625+
oldString = newString;
626+
newString = interpolateFn.compute(newVals);
627+
attr.$set('value', newString);
628+
if (oldString) selectCtrl.removeOption(oldString);
629+
selectCtrl.addOption(newString);
626630
});
627631
} else {
628632
selectCtrl.addOption(attr.value);

‎src/ng/interpolate.js‎

Lines changed: 49 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -114,12 +114,15 @@ function $InterpolateProvider() {
114114
* result through {@link ng.$sce#getTrusted $sce.getTrusted(interpolatedResult,
115115
* trustedContext)} before returning it. Refer to the {@link ng.$sce $sce} service that
116116
* provides Strict Contextual Escaping for details.
117-
* @returns {function(context)} an interpolation function which is used to compute the
118-
* interpolated string. The function has these parameters:
117+
* @returns {Object} An object describing the interpolation template string.
119118
*
120-
* * `context`: an object against which any expressions embedded in the strings are evaluated
121-
* against.
119+
* The properties of the returned object include:
122120
*
121+
* - `template` — `{string}` — original interpolation template string.
122+
* - `separators` — `{Array.<string>}` — array of separators extracted from the template.
123+
* - `expressions` — `{Array.<string>}` — array of expressions extracted from the template.
124+
* - `compute` — {function(Array)()} — function that when called with an array of values will
125+
* compute the result of interpolation for the given interpolation template and values.
123126
*/
124127
function $interpolate(text, mustHaveExpression, trustedContext) {
125128
var startIndex,
@@ -139,8 +142,8 @@ function $InterpolateProvider() {
139142
((endIndex = text.indexOf(endSymbol, startIndex + startSymbolLength)) != -1) ) {
140143
if (index !== startIndex) hasText = true;
141144
separators.push(text.substring(index, startIndex));
142-
expressions.push(fn = interpolateParse(exp = text.substring(startIndex + startSymbolLength, endIndex)));
143-
fn.exp = exp;
145+
exp = text.substring(startIndex + startSymbolLength, endIndex);
146+
expressions.push(exp);
144147
index = endIndex + endSymbolLength;
145148
hasInterpolation = true;
146149
} else {
@@ -172,55 +175,51 @@ function $InterpolateProvider() {
172175

173176
if (!mustHaveExpression || hasInterpolation) {
174177
concat.length = separators.length + expressions.length;
175-
var computeFn = function (values, context) {
176-
for(var i = 0, ii = expressions.length; i < ii; i++) {
177-
concat[2*i] = separators[i];
178-
concat[(2*i)+1] = values ? values[i] : expressions[i](context);
178+
179+
return extend(function interpolationFn(scope) {
180+
var values = [];
181+
forEach(interpolationFn.expressions, function(expression) {
182+
values.push(scope.$eval(expression));
183+
});
184+
return interpolationFn.compute(values);
185+
}, {
186+
exp: text, //deprecated
187+
template: text,
188+
separators: separators,
189+
expressions: expressions,
190+
compute: function(values) {
191+
for(var i = 0, ii = expressions.length; i < ii; i++) {
192+
concat[2*i] = separators[i];
193+
concat[(2*i)+1] = stringify(values[i]);
194+
}
195+
concat[2*ii] = separators[ii];
196+
return concat.join('');
179197
}
180-
concat[2*ii] = separators[ii];
181-
return concat.join('');
182-
};
198+
});
199+
}
183200

184-
fn = function(context) {
185-
return computeFn(null, context);
186-
};
187-
fn.exp = text;
201+
function stringify(value) {
202+
try {
188203

189-
// hack in order to preserve existing api
190-
fn.$$invoke = function (listener) {
191-
return function (values, oldValues, scope) {
192-
var current = computeFn(values, scope);
193-
listener(current, this.$$lastInter == null ? current : this.$$lastInter, scope);
194-
this.$$lastInter = current;
195-
};
196-
};
197-
fn.separators = separators;
198-
fn.expressions = expressions;
199-
return fn;
200-
}
204+
if (trustedContext) {
205+
value = $sce.getTrusted(trustedContext, value);
206+
} else {
207+
value = $sce.valueOf(value);
208+
}
201209

202-
function interpolateParse(expression) {
203-
var exp = $parse(expression);
204-
return function (scope) {
205-
try {
206-
var value = exp(scope);
207-
if (trustedContext) {
208-
value = $sce.getTrusted(trustedContext, value);
209-
} else {
210-
value = $sce.valueOf(value);
211-
}
212-
if (value === null || isUndefined(value)) {
213-
value = '';
214-
} else if (typeof value != 'string') {
215-
value = toJson(value);
216-
}
217-
return value;
218-
} catch(err) {
219-
var newErr = $interpolateMinErr('interr', "Can't interpolate: {0}\n{1}", text,
220-
err.toString());
221-
$exceptionHandler(newErr);
210+
if (value === null || isUndefined(value)) {
211+
value = '';
212+
} else if (typeof value != 'string') {
213+
value = toJson(value);
222214
}
223-
};
215+
216+
return value;
217+
218+
} catch(err) {
219+
var newErr = $interpolateMinErr('interr', "Can't interpolate: {0}\n{1}", text,
220+
err.toString());
221+
$exceptionHandler(newErr);
222+
}
224223
}
225224
}
226225

‎src/ngScenario/Scenario.js‎

Lines changed: 17 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -302,27 +302,24 @@ _jQuery.fn.bindings = function(windowJquery, bindExp) {
302302

303303
selection.each(function() {
304304
var element = windowJquery(this),
305-
binding;
306-
if (binding = element.data('$binding')) {
307-
if (typeof binding == 'string') {
308-
if (match(binding)) {
309-
push(element.scope().$eval(binding));
310-
}
311-
} else {
312-
if (!angular.isArray(binding)) {
313-
binding = [binding];
305+
bindings;
306+
if (bindings = element.data('$binding')) {
307+
if (!angular.isArray(bindings)) {
308+
bindings = [bindings];
309+
}
310+
for(var expressions = [], binding, j=0, jj=bindings.length; j<jj; j++) {
311+
binding = bindings[j];
312+
313+
if (binding.expressions) {
314+
expressions = binding.expressions;
315+
} else {
316+
expressions = [binding];
314317
}
315-
for(var fns, j=0, jj=binding.length; j<jj; j++) {
316-
fns = binding[j];
317-
if (fns.expressions) {
318-
fns = fns.expressions;
319-
} else {
320-
fns = [fns];
321-
}
322-
for (var scope, fn, i = 0, ii = fns.length; i < ii; i++) {
323-
if(match((fn = fns[i]).exp)) {
324-
push(fn(scope = scope || element.scope()));
325-
}
318+
for (var scope, expression, i = 0, ii = expressions.length; i < ii; i++) {
319+
expression = expressions[i];
320+
if(match(expression)) {
321+
scope = scope || element.scope();
322+
push(scope.$eval(expression));
326323
}
327324
}
328325
}

‎test/BinderSpec.js‎

Lines changed: 0 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -167,30 +167,6 @@ describe('Binder', function() {
167167
expect(element[0].childNodes.length).toEqual(1);
168168
}));
169169

170-
it('IfTextBindingThrowsErrorDecorateTheSpan', function() {
171-
module(function($exceptionHandlerProvider){
172-
$exceptionHandlerProvider.mode('log');
173-
});
174-
inject(function($rootScope, $exceptionHandler, $compile) {
175-
element = $compile('<div>{{error.throw()}}</div>', null, true)($rootScope);
176-
var errorLogs = $exceptionHandler.errors;
177-
178-
$rootScope.error = {
179-
'throw': function() {throw 'ErrorMsg1';}
180-
};
181-
$rootScope.$apply();
182-
183-
$rootScope.error['throw'] = function() {throw 'MyError';};
184-
errorLogs.length = 0;
185-
$rootScope.$apply();
186-
expect(errorLogs.shift().message).toMatch(/^\[\$interpolate:interr\] Can't interpolate: \{\{error.throw\(\)\}\}\nMyError/);
187-
188-
$rootScope.error['throw'] = function() {return 'ok';};
189-
$rootScope.$apply();
190-
expect(errorLogs.length).toBe(0);
191-
});
192-
});
193-
194170
it('IfAttrBindingThrowsErrorDecorateTheAttribute', function() {
195171
module(function($exceptionHandlerProvider){
196172
$exceptionHandlerProvider.mode('log');

‎test/ng/compileSpec.js‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2080,6 +2080,8 @@ describe('$compile', function() {
20802080

20812081

20822082
it('should set interpolated attrs to initial interpolation value', inject(function($rootScope, $compile) {
2083+
// we need the interpolated attributes to be initialized so that linking fn in a component
2084+
// can access the value during link
20832085
$rootScope.whatever = 'test value';
20842086
$compile('<div some-attr="{{whatever}}" observer></div>')($rootScope);
20852087
expect(directiveAttrs.someAttr).toBe($rootScope.whatever);

0 commit comments

Comments
 (0)