Skip to content
This repository has been archived by the owner on Apr 12, 2024. It is now read-only.

Commit

Permalink
refactor($interpolate): attempt to remove hacky code due to $interpol…
Browse files Browse the repository at this point in the history 
…ation perf improvements
  • Loading branch information
@IgorMinar
IgorMinar committed Apr 21, 2014
1 parent 0ebfa0d commit 1db3b8c
Show file tree
Hide file tree
Showing 9 changed files with 172 additions and 186 deletions.
31 changes: 20 additions & 11 deletions src/ng/compile.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});
})
});
}
Expand Down Expand Up @@ -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',
Expand All @@ -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);
}
}));
});
}
};
}
Expand Down
2 changes: 1 addition & 1 deletion src/ng/directive/ngPluralize.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 '';
}
Expand Down
12 changes: 8 additions & 4 deletions src/ng/directive/select.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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);
Expand Down
99 changes: 49 additions & 50 deletions src/ng/interpolate.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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 {
Expand Down Expand Up @@ -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);
}
}
}

Expand Down
37 changes: 17 additions & 20 deletions src/ngScenario/Scenario.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<jj; j++) {
binding = bindings[j];

if (binding.expressions) {
expressions = binding.expressions;
} else {
expressions = [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));
}
}
}
Expand Down
24 changes: 0 additions & 24 deletions test/BinderSpec.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand Down
2 changes: 2 additions & 0 deletions test/ng/compileSpec.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
Loading

0 comments on commit 1db3b8c

Please sign in to comment.