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

Commit

Permalink
fix($parse): evaluate once simple expressions in interpolations
Browse files Browse the repository at this point in the history
For simple expressions without filters that have a stateless interceptor
then handle the 2nd phase parse evaluation using `inputs`.

TL;DR
This fixes the issue that interpolated simple expressions were evaluated twice
within one digest loop.

Long version, things happen in the following order:

* There was an overhaul on $interpolate, this overhaul changed $parse and
  incorporated the concept of an interceptor.
* Optimization on $parse landed  so expressions that have filters without
  parameters (or the parameters are constants) would be evaluated in 2 phases,
  first to evaluate the expression sans the filter evaluation and then with
  the filter evaluation. This also used interceptors [the second evaluation
  issue was added here]
* More optimizations on $parse landed and now expressions could be evaluated
  in 2 phases. One to get all the possible values that could change (lets call
  this state), the state was checked by $watch to know if an expression changed.
  The second to continue the evaluation (as long as this state is provided).
  This, once again, used interceptors

The last change, was supposed to fix the issue, but there was an assumption in
the existing code that the code would always generate the 2 phases functions,
but that is not true. If the expression is simple enough (just like the one in
your case) then the 2-phase evaluations functions are not generated. In this
case, if a stateless interceptor was added (just like what $interpolate adds)
then the state was not used and you see the function being evaluated twice.
This explains why, if you change the expression from
`Hello {{log('A')}} {{log('B')}}!` to `Hello {{log('A') + ' ' + log('B')}}!`,
then the repetition is not there.

Closes #12983
Closes #13002
  • Loading branch information
lgalfaso committed Oct 14, 2015
1 parent 9f716dd commit 1caf0b6
Show file tree
Hide file tree
Showing 2 changed files with 11 additions and 1 deletion.
4 changes: 3 additions & 1 deletion src/ng/parse.js
Original file line number Diff line number Diff line change
Expand Up @@ -1906,13 +1906,14 @@ function $ParseProvider() {
function addInterceptor(parsedExpression, interceptorFn) {
if (!interceptorFn) return parsedExpression;
var watchDelegate = parsedExpression.$$watchDelegate;
var useInputs = false;

var regularWatch =
watchDelegate !== oneTimeLiteralWatchDelegate &&
watchDelegate !== oneTimeWatchDelegate;

var fn = regularWatch ? function regularInterceptedExpression(scope, locals, assign, inputs) {
var value = parsedExpression(scope, locals, assign, inputs);
var value = useInputs && inputs ? inputs[0] : parsedExpression(scope, locals, assign, inputs);
return interceptorFn(value, scope, locals);
} : function oneTimeInterceptedExpression(scope, locals, assign, inputs) {
var value = parsedExpression(scope, locals, assign, inputs);
Expand All @@ -1930,6 +1931,7 @@ function $ParseProvider() {
// If there is an interceptor, but no watchDelegate then treat the interceptor like
// we treat filters - it is assumed to be a pure function unless flagged with $stateful
fn.$$watchDelegate = inputsWatchDelegate;
useInputs = !parsedExpression.inputs;
fn.inputs = parsedExpression.inputs ? parsedExpression.inputs : [parsedExpression];
}

Expand Down
8 changes: 8 additions & 0 deletions test/ng/compileSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -947,6 +947,14 @@ describe('$compile', function() {
expect(child).toHaveClass('log'); // merged from replace directive template
}));

it('should interpolate the values once per digest',
inject(function($compile, $rootScope, log) {
element = $compile('<div>{{log("A")}} foo {{::log("B")}}</div>')($rootScope);
$rootScope.log = log;
$rootScope.$digest();
expect(log).toEqual('A; B; A; B');
}));

it('should update references to replaced jQuery context', function() {
module(function($compileProvider) {
$compileProvider.directive('foo', function() {
Expand Down

4 comments on commit 1caf0b6

@Narretz
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be backported to 1.4?

@lgalfaso
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Narretz I do not think there is a need, but would not oppose for this to be backported

@Narretz
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't we usually backport all fixes to the stable version?

@lgalfaso
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

backported to 1.4 as e403682

Please sign in to comment.