Skip to content

Commit c63d7e6

Browse files
jbedardjbedard
committed
fix($parse): always re-evaluate filters within literals when an input is an object
Fixes angular#15964
1 parent cd5efa0 commit c63d7e6

File tree

2 files changed

+175
-27
lines changed

2 files changed

+175
-27
lines changed

‎src/ng/parse.js

+68-25
Original file line numberDiff line numberDiff line change
@@ -622,15 +622,51 @@ function isStateless($filter, filterName) {
622622
return !fn.$stateful;
623623
}
624624

625-
function findConstantAndWatchExpressions(ast, $filter) {
625+
// Detect nodes which could depend on non-shallow state of objects
626+
function isShallowState(stack) {
627+
for (var i = 0; i < stack.length; i++) {
628+
var node = stack[i];
629+
switch (node.type) {
630+
// Computed members might invoke a stateful toString()
631+
case AST.MemberExpression:
632+
if (node.computed) {
633+
return false;
634+
}
635+
break;
636+
637+
// The + operator can invoke a stateful toString().
638+
// Otherwise operators can not access any state within objects.
639+
case AST.UnaryExpression:
640+
case AST.BinaryExpression:
641+
if (node.operator === '+') {
642+
return false;
643+
}
644+
break;
645+
646+
// Functions / filters probably read state from within objects
647+
case AST.CallExpression:
648+
return false;
649+
}
650+
}
651+
652+
return true;
653+
}
654+
655+
function findConstantAndWatchExpressions(ast, $filter, stack) {
626656
var allConstants;
627657
var argsToWatch;
628658
var isStatelessFilter;
659+
660+
stack = stack || [];
661+
stack.push(ast);
662+
663+
ast.isShallowState = isShallowState(stack);
664+
629665
switch (ast.type) {
630666
case AST.Program:
631667
allConstants = true;
632668
forEach(ast.body, function(expr) {
633-
findConstantAndWatchExpressions(expr.expression, $filter);
669+
findConstantAndWatchExpressions(expr.expression, $filter, stack);
634670
allConstants = allConstants && expr.expression.constant;
635671
});
636672
ast.constant = allConstants;
@@ -640,26 +676,26 @@ function findConstantAndWatchExpressions(ast, $filter) {
640676
ast.toWatch = [];
641677
break;
642678
case AST.UnaryExpression:
643-
findConstantAndWatchExpressions(ast.argument, $filter);
679+
findConstantAndWatchExpressions(ast.argument, $filter, stack);
644680
ast.constant = ast.argument.constant;
645681
ast.toWatch = ast.argument.toWatch;
646682
break;
647683
case AST.BinaryExpression:
648-
findConstantAndWatchExpressions(ast.left, $filter);
649-
findConstantAndWatchExpressions(ast.right, $filter);
684+
findConstantAndWatchExpressions(ast.left, $filter, stack);
685+
findConstantAndWatchExpressions(ast.right, $filter, stack);
650686
ast.constant = ast.left.constant && ast.right.constant;
651687
ast.toWatch = ast.left.toWatch.concat(ast.right.toWatch);
652688
break;
653689
case AST.LogicalExpression:
654-
findConstantAndWatchExpressions(ast.left, $filter);
655-
findConstantAndWatchExpressions(ast.right, $filter);
690+
findConstantAndWatchExpressions(ast.left, $filter, stack);
691+
findConstantAndWatchExpressions(ast.right, $filter, stack);
656692
ast.constant = ast.left.constant && ast.right.constant;
657693
ast.toWatch = ast.constant ? [] : [ast];
658694
break;
659695
case AST.ConditionalExpression:
660-
findConstantAndWatchExpressions(ast.test, $filter);
661-
findConstantAndWatchExpressions(ast.alternate, $filter);
662-
findConstantAndWatchExpressions(ast.consequent, $filter);
696+
findConstantAndWatchExpressions(ast.test, $filter, stack);
697+
findConstantAndWatchExpressions(ast.alternate, $filter, stack);
698+
findConstantAndWatchExpressions(ast.consequent, $filter, stack);
663699
ast.constant = ast.test.constant && ast.alternate.constant && ast.consequent.constant;
664700
ast.toWatch = ast.constant ? [] : [ast];
665701
break;
@@ -668,9 +704,9 @@ function findConstantAndWatchExpressions(ast, $filter) {
668704
ast.toWatch = [ast];
669705
break;
670706
case AST.MemberExpression:
671-
findConstantAndWatchExpressions(ast.object, $filter);
707+
findConstantAndWatchExpressions(ast.object, $filter, stack);
672708
if (ast.computed) {
673-
findConstantAndWatchExpressions(ast.property, $filter);
709+
findConstantAndWatchExpressions(ast.property, $filter, stack);
674710
}
675711
ast.constant = ast.object.constant && (!ast.computed || ast.property.constant);
676712
ast.toWatch = [ast];
@@ -680,7 +716,7 @@ function findConstantAndWatchExpressions(ast, $filter) {
680716
allConstants = isStatelessFilter;
681717
argsToWatch = [];
682718
forEach(ast.arguments, function(expr) {
683-
findConstantAndWatchExpressions(expr, $filter);
719+
findConstantAndWatchExpressions(expr, $filter, stack);
684720
allConstants = allConstants && expr.constant;
685721
if (!expr.constant) {
686722
argsToWatch.push.apply(argsToWatch, expr.toWatch);
@@ -690,16 +726,16 @@ function findConstantAndWatchExpressions(ast, $filter) {
690726
ast.toWatch = isStatelessFilter ? argsToWatch : [ast];
691727
break;
692728
case AST.AssignmentExpression:
693-
findConstantAndWatchExpressions(ast.left, $filter);
694-
findConstantAndWatchExpressions(ast.right, $filter);
729+
findConstantAndWatchExpressions(ast.left, $filter, stack);
730+
findConstantAndWatchExpressions(ast.right, $filter, stack);
695731
ast.constant = ast.left.constant && ast.right.constant;
696732
ast.toWatch = [ast];
697733
break;
698734
case AST.ArrayExpression:
699735
allConstants = true;
700736
argsToWatch = [];
701737
forEach(ast.elements, function(expr) {
702-
findConstantAndWatchExpressions(expr, $filter);
738+
findConstantAndWatchExpressions(expr, $filter, stack);
703739
allConstants = allConstants && expr.constant;
704740
if (!expr.constant) {
705741
argsToWatch.push.apply(argsToWatch, expr.toWatch);
@@ -712,13 +748,13 @@ function findConstantAndWatchExpressions(ast, $filter) {
712748
allConstants = true;
713749
argsToWatch = [];
714750
forEach(ast.properties, function(property) {
715-
findConstantAndWatchExpressions(property.value, $filter);
751+
findConstantAndWatchExpressions(property.value, $filter, stack);
716752
allConstants = allConstants && property.value.constant && !property.computed;
717753
if (!property.value.constant) {
718754
argsToWatch.push.apply(argsToWatch, property.value.toWatch);
719755
}
720756
if (property.computed) {
721-
findConstantAndWatchExpressions(property.key, $filter);
757+
findConstantAndWatchExpressions(property.key, $filter, stack);
722758
if (!property.key.constant) {
723759
argsToWatch.push.apply(argsToWatch, property.key.toWatch);
724760
}
@@ -737,6 +773,8 @@ function findConstantAndWatchExpressions(ast, $filter) {
737773
ast.toWatch = [];
738774
break;
739775
}
776+
777+
stack.pop();
740778
}
741779

742780
function getInputs(body) {
@@ -781,7 +819,7 @@ ASTCompiler.prototype = {
781819
filters: {},
782820
fn: {vars: [], body: [], own: {}},
783821
assign: {vars: [], body: [], own: {}},
784-
inputs: []
822+
inputs: {}
785823
};
786824
findConstantAndWatchExpressions(ast, self.$filter);
787825
var extra = '';
@@ -803,7 +841,7 @@ ASTCompiler.prototype = {
803841
var intoId = self.nextId();
804842
self.recurse(watch, intoId);
805843
self.return_(intoId);
806-
self.state.inputs.push(fnKey);
844+
self.state.inputs[fnKey] = !!watch.isShallowState;
807845
watch.watchId = key;
808846
});
809847
this.state.computing = 'fn';
@@ -840,12 +878,16 @@ ASTCompiler.prototype = {
840878
watchFns: function() {
841879
var result = [];
842880
var fns = this.state.inputs;
881+
var fnNames = Object.keys(fns);
843882
var self = this;
844-
forEach(fns, function(name) {
883+
forEach(fnNames, function(name) {
845884
result.push('var ' + name + '=' + self.generateFunction(name, 's'));
885+
if (fns[name]) {
886+
result.push(name, ".isShallowState=true;");
887+
}
846888
});
847-
if (fns.length) {
848-
result.push('fn.inputs=[' + fns.join(',') + '];');
889+
if (fnNames.length) {
890+
result.push('fn.inputs=[' + fnNames.join(',') + '];');
849891
}
850892
return result.join('');
851893
},
@@ -1251,6 +1293,7 @@ ASTInterpreter.prototype = {
12511293
inputs = [];
12521294
forEach(toWatch, function(watch, key) {
12531295
var input = self.recurse(watch);
1296+
input.isShallowState = watch.isShallowState;
12541297
watch.input = input;
12551298
inputs.push(input);
12561299
watch.watchId = key;
@@ -1817,7 +1860,7 @@ function $ParseProvider() {
18171860
inputExpressions = inputExpressions[0];
18181861
return scope.$watch(function expressionInputWatch(scope) {
18191862
var newInputValue = inputExpressions(scope);
1820-
if (!expressionInputDirtyCheck(newInputValue, oldInputValueOf, parsedExpression.literal)) {
1863+
if (!expressionInputDirtyCheck(newInputValue, oldInputValueOf, inputExpressions.isShallowState)) {
18211864
lastResult = parsedExpression(scope, undefined, undefined, [newInputValue]);
18221865
oldInputValueOf = newInputValue && getValueOf(newInputValue);
18231866
}
@@ -1837,7 +1880,7 @@ function $ParseProvider() {
18371880

18381881
for (var i = 0, ii = inputExpressions.length; i < ii; i++) {
18391882
var newInputValue = inputExpressions[i](scope);
1840-
if (changed || (changed = !expressionInputDirtyCheck(newInputValue, oldInputValueOfValues[i], parsedExpression.literal))) {
1883+
if (changed || (changed = !expressionInputDirtyCheck(newInputValue, oldInputValueOfValues[i], inputExpressions[i].isShallowState))) {
18411884
oldInputValues[i] = newInputValue;
18421885
oldInputValueOfValues[i] = newInputValue && getValueOf(newInputValue);
18431886
}

‎test/ng/parseSpec.js

+107-2
Original file line numberDiff line numberDiff line change
@@ -2973,6 +2973,111 @@ describe('parser', function() {
29732973
expect(watcherCalls).toBe(1);
29742974
}));
29752975

2976+
it('should not reevaluate filters in literals with non-primitive input that does support valueOf()',
2977+
inject(function($parse) {
2978+
var filterCalls = 0;
2979+
$filterProvider.register('foo', valueFn(function(input) {
2980+
filterCalls++;
2981+
return input;
2982+
}));
2983+
2984+
scope.date = new Date(1234567890123);
2985+
2986+
var watcherCalls = 0;
2987+
scope.$watch('[(date | foo)]', function(input) {
2988+
watcherCalls++;
2989+
});
2990+
2991+
scope.$digest();
2992+
expect(filterCalls).toBe(1);
2993+
expect(watcherCalls).toBe(1);
2994+
2995+
scope.$digest();
2996+
expect(filterCalls).toBe(1);
2997+
expect(watcherCalls).toBe(1);
2998+
}));
2999+
3000+
it('should reevaluate filters in literals with non-primitive input that does support valueOf() when' +
3001+
' valueOf() value changes',
3002+
inject(function($parse) {
3003+
var filterCalls = 0;
3004+
$filterProvider.register('foo', valueFn(function(input) {
3005+
filterCalls++;
3006+
return input;
3007+
}));
3008+
3009+
scope.date = new Date(1234567890123);
3010+
3011+
var watcherCalls = 0;
3012+
scope.$watch('[(date | foo)]', function(input) {
3013+
watcherCalls++;
3014+
});
3015+
3016+
scope.$digest();
3017+
expect(filterCalls).toBe(1);
3018+
expect(watcherCalls).toBe(1);
3019+
3020+
scope.date.setTime(1234567890);
3021+
3022+
scope.$digest();
3023+
expect(filterCalls).toBe(2);
3024+
expect(watcherCalls).toBe(2);
3025+
}));
3026+
3027+
it('should not reevaluate literals containing filters with non-primitive input that does support valueOf()' +
3028+
' when the instance changes but valueOf() does not', inject(function($parse) {
3029+
var filterCalls = 0;
3030+
$filterProvider.register('foo', valueFn(function(input) {
3031+
filterCalls++;
3032+
return input;
3033+
}));
3034+
3035+
scope.date = new Date(1234567890123);
3036+
3037+
var watcherCalls = 0;
3038+
scope.$watch($parse('[(date | foo)]'), function(input) {
3039+
watcherCalls++;
3040+
});
3041+
3042+
scope.$digest();
3043+
expect(watcherCalls).toBe(1);
3044+
3045+
scope.date = new Date(1234567890123);
3046+
scope.$digest();
3047+
expect(watcherCalls).toBe(1);
3048+
}));
3049+
3050+
it('should not reevaluate filters with literal input containing primitives', inject(function($parse) {
3051+
var filterCalls = 0;
3052+
$filterProvider.register('foo', valueFn(function(input) {
3053+
filterCalls++;
3054+
return input;
3055+
}));
3056+
3057+
var watcherCalls = 0;
3058+
scope.$watch('[a] | foo', function(input) {
3059+
watcherCalls++;
3060+
});
3061+
3062+
scope.$apply("a = 1");
3063+
expect(filterCalls).toBe(1);
3064+
expect(watcherCalls).toBe(1);
3065+
3066+
scope.$apply("a = 2");
3067+
expect(filterCalls).toBe(2);
3068+
expect(watcherCalls).toBe(2);
3069+
}));
3070+
3071+
it('should always reevaluate filters with literal input containing non-primitives',
3072+
inject(function($parse) {
3073+
scope.$watch('[a] | filter', function() {});
3074+
3075+
scope.$apply("a = 1");
3076+
3077+
// Would be great if this didn't throw, but this is the best way to verify atm...
3078+
expect(function() { scope.$apply("a = {}"); }).toThrowMinErr('$rootScope', 'infdig');
3079+
}));
3080+
29763081
it('should always reevaluate filters with non-primitive input created with null prototype',
29773082
inject(function($parse) {
29783083
var filterCalls = 0;
@@ -3027,7 +3132,7 @@ describe('parser', function() {
30273132
}));
30283133

30293134
it('should reevaluate filters with non-primitive input that does support valueOf() when' +
3030-
'valueOf() value changes', inject(function($parse) {
3135+
' valueOf() value changes', inject(function($parse) {
30313136
var filterCalls = 0;
30323137
$filterProvider.register('foo', valueFn(function(input) {
30333138
filterCalls++;
@@ -3055,7 +3160,7 @@ describe('parser', function() {
30553160
expect(watcherCalls).toBe(1);
30563161
}));
30573162

3058-
it('should invoke interceptorFns if they are flagged as having $stateful',
3163+
it('should always invoke interceptorFns if they are flagged as having $stateful',
30593164
inject(function($parse) {
30603165
var called = false;
30613166
function interceptor() {

0 commit comments

Comments
 (0)