Skip to content

Commit 189461f

Browse files
jbedardNarretz
jbedard
authored and
Narretz
committed
fix($parse): standardize one-time literal vs non-literal and interceptors
Previously literal one-time bindings did not use the expression `inputs`, causing infinite digest issues with literal values. This often forces the use of deepEquals when watching one-time literals. `ng-class` is one example of deepEquals which is no longer required. This one-time/literal behavior is now also consistently propogated through interceptors. Closes angular#15858
1 parent 93879b3 commit 189461f

File tree

3 files changed

+114
-159
lines changed

3 files changed

+114
-159
lines changed

‎src/ng/directive/ngClass.js‎

Lines changed: 8 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -14,13 +14,6 @@ function classDirective(name, selector) {
1414
return {
1515
restrict: 'AC',
1616
link: function(scope, element, attr) {
17-
var expression = attr[name].trim();
18-
var isOneTime = (expression.charAt(0) === ':') && (expression.charAt(1) === ':');
19-
20-
var watchInterceptor = isOneTime ? toFlatValue : toClassString;
21-
var watchExpression = $parse(expression, watchInterceptor);
22-
var watchAction = isOneTime ? ngClassOneTimeWatchAction : ngClassWatchAction;
23-
2417
var classCounts = element.data('$classCounts');
2518
var oldModulo = true;
2619
var oldClassString;
@@ -43,7 +36,7 @@ function classDirective(name, selector) {
4336
scope.$watch(indexWatchExpression, ngClassIndexWatchAction);
4437
}
4538

46-
scope.$watch(watchExpression, watchAction, isOneTime);
39+
scope.$watch($parse(attr[name], toClassString), ngClassWatchAction);
4740

4841
function addClasses(classString) {
4942
classString = digestClassCounts(split(classString), 1);
@@ -85,9 +78,9 @@ function classDirective(name, selector) {
8578
}
8679

8780
function ngClassIndexWatchAction(newModulo) {
88-
// This watch-action should run before the `ngClass[OneTime]WatchAction()`, thus it
81+
// This watch-action should run before the `ngClassWatchAction()`, thus it
8982
// adds/removes `oldClassString`. If the `ngClass` expression has changed as well, the
90-
// `ngClass[OneTime]WatchAction()` will update the classes.
83+
// `ngClassWatchAction()` will update the classes.
9184
if (newModulo === selector) {
9285
addClasses(oldClassString);
9386
} else {
@@ -97,15 +90,13 @@ function classDirective(name, selector) {
9790
oldModulo = newModulo;
9891
}
9992

100-
function ngClassOneTimeWatchAction(newClassValue) {
101-
var newClassString = toClassString(newClassValue);
102-
103-
if (newClassString !== oldClassString) {
104-
ngClassWatchAction(newClassString);
93+
function ngClassWatchAction(newClassString) {
94+
// When using a one-time binding the newClassString will return
95+
// the pre-interceptor value until the one-time is complete
96+
if (!isString(newClassString)) {
97+
newClassString = toClassString(newClassString);
10598
}
106-
}
10799

108-
function ngClassWatchAction(newClassString) {
109100
if (oldModulo === selector) {
110101
updateClasses(oldClassString, newClassString);
111102
}
@@ -152,34 +143,6 @@ function classDirective(name, selector) {
152143

153144
return classString;
154145
}
155-
156-
function toFlatValue(classValue) {
157-
var flatValue = classValue;
158-
159-
if (isArray(classValue)) {
160-
flatValue = classValue.map(toFlatValue);
161-
} else if (isObject(classValue)) {
162-
var hasUndefined = false;
163-
164-
flatValue = Object.keys(classValue).filter(function(key) {
165-
var value = classValue[key];
166-
167-
if (!hasUndefined && isUndefined(value)) {
168-
hasUndefined = true;
169-
}
170-
171-
return value;
172-
});
173-
174-
if (hasUndefined) {
175-
// Prevent the `oneTimeLiteralWatchInterceptor` from unregistering
176-
// the watcher, by including at least one `undefined` value.
177-
flatValue.push(undefined);
178-
}
179-
}
180-
181-
return flatValue;
182-
}
183146
}
184147

185148
/**

‎src/ng/parse.js‎

Lines changed: 26 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -1765,8 +1765,8 @@ function $ParseProvider() {
17651765
if (parsedExpression.constant) {
17661766
parsedExpression.$$watchDelegate = constantWatchDelegate;
17671767
} else if (oneTime) {
1768-
parsedExpression.$$watchDelegate = parsedExpression.literal ?
1769-
oneTimeLiteralWatchDelegate : oneTimeWatchDelegate;
1768+
parsedExpression.oneTime = true;
1769+
parsedExpression.$$watchDelegate = oneTimeWatchDelegate;
17701770
} else if (parsedExpression.inputs) {
17711771
parsedExpression.$$watchDelegate = inputsWatchDelegate;
17721772
}
@@ -1852,6 +1852,7 @@ function $ParseProvider() {
18521852
}
18531853

18541854
function oneTimeWatchDelegate(scope, listener, objectEquality, parsedExpression, prettyPrintExpression) {
1855+
var isDone = parsedExpression.literal ? isAllDefined : isDefined;
18551856
var unwatch, lastValue;
18561857
if (parsedExpression.inputs) {
18571858
unwatch = inputsWatchDelegate(scope, oneTimeListener, objectEquality, parsedExpression, prettyPrintExpression);
@@ -1868,41 +1869,22 @@ function $ParseProvider() {
18681869
if (isFunction(listener)) {
18691870
listener(value, old, scope);
18701871
}
1871-
if (isDefined(value)) {
1872+
if (isDone(value)) {
18721873
scope.$$postDigest(function() {
1873-
if (isDefined(lastValue)) {
1874+
if (isDone(lastValue)) {
18741875
unwatch();
18751876
}
18761877
});
18771878
}
18781879
}
18791880
}
18801881

1881-
function oneTimeLiteralWatchDelegate(scope, listener, objectEquality, parsedExpression) {
1882-
var unwatch, lastValue;
1883-
unwatch = scope.$watch(function oneTimeWatch(scope) {
1884-
return parsedExpression(scope);
1885-
}, function oneTimeListener(value, old, scope) {
1886-
lastValue = value;
1887-
if (isFunction(listener)) {
1888-
listener(value, old, scope);
1889-
}
1890-
if (isAllDefined(value)) {
1891-
scope.$$postDigest(function() {
1892-
if (isAllDefined(lastValue)) unwatch();
1893-
});
1894-
}
1895-
}, objectEquality);
1896-
1897-
return unwatch;
1898-
1899-
function isAllDefined(value) {
1900-
var allDefined = true;
1901-
forEach(value, function(val) {
1902-
if (!isDefined(val)) allDefined = false;
1903-
});
1904-
return allDefined;
1905-
}
1882+
function isAllDefined(value) {
1883+
var allDefined = true;
1884+
forEach(value, function(val) {
1885+
if (!isDefined(val)) allDefined = false;
1886+
});
1887+
return allDefined;
19061888
}
19071889

19081890
function constantWatchDelegate(scope, listener, objectEquality, parsedExpression) {
@@ -1918,22 +1900,28 @@ function $ParseProvider() {
19181900
var watchDelegate = parsedExpression.$$watchDelegate;
19191901
var useInputs = false;
19201902

1921-
var regularWatch =
1922-
watchDelegate !== oneTimeLiteralWatchDelegate &&
1923-
watchDelegate !== oneTimeWatchDelegate;
1903+
var isDone = parsedExpression.literal ? isAllDefined : isDefined;
19241904

1925-
var fn = regularWatch ? function regularInterceptedExpression(scope, locals, assign, inputs) {
1905+
function regularInterceptedExpression(scope, locals, assign, inputs) {
19261906
var value = useInputs && inputs ? inputs[0] : parsedExpression(scope, locals, assign, inputs);
19271907
return interceptorFn(value, scope, locals);
1928-
} : function oneTimeInterceptedExpression(scope, locals, assign, inputs) {
1929-
var value = parsedExpression(scope, locals, assign, inputs);
1908+
}
1909+
1910+
function oneTimeInterceptedExpression(scope, locals, assign, inputs) {
1911+
var value = useInputs && inputs ? inputs[0] : parsedExpression(scope, locals, assign, inputs);
19301912
var result = interceptorFn(value, scope, locals);
19311913
// we only return the interceptor's result if the
19321914
// initial value is defined (for bind-once)
1933-
return isDefined(value) ? result : value;
1934-
};
1915+
return isDone(value) ? result : value;
1916+
}
1917+
1918+
var fn = parsedExpression.oneTime ? oneTimeInterceptedExpression : regularInterceptedExpression;
1919+
1920+
// Propogate the literal/oneTime attributes
1921+
fn.literal = parsedExpression.literal;
1922+
fn.oneTime = parsedExpression.oneTime;
19351923

1936-
// Propagate $$watchDelegates other then inputsWatchDelegate
1924+
// Propagate or create inputs / $$watchDelegates
19371925
useInputs = !parsedExpression.inputs;
19381926
if (watchDelegate && watchDelegate !== inputsWatchDelegate) {
19391927
fn.$$watchDelegate = watchDelegate;

‎test/ng/parseSpec.js‎

Lines changed: 80 additions & 76 deletions
Original file line numberDiff line numberDiff line change
@@ -2688,82 +2688,86 @@ describe('parser', function() {
26882688
expect($parse(':: ').literal).toBe(true);
26892689
}));
26902690

2691-
it('should only become stable when all the properties of an object have defined values', inject(function($parse, $rootScope, log) {
2692-
var fn = $parse('::{foo: foo, bar: bar}');
2693-
$rootScope.$watch(fn, function(value) { log(value); }, true);
2694-
2695-
expect(log.empty()).toEqual([]);
2696-
expect($rootScope.$$watchers.length).toBe(1);
2697-
2698-
$rootScope.$digest();
2699-
expect($rootScope.$$watchers.length).toBe(1);
2700-
expect(log.empty()).toEqual([{foo: undefined, bar: undefined}]);
2701-
2702-
$rootScope.foo = 'foo';
2703-
$rootScope.$digest();
2704-
expect($rootScope.$$watchers.length).toBe(1);
2705-
expect(log.empty()).toEqual([{foo: 'foo', bar: undefined}]);
2706-
2707-
$rootScope.foo = 'foobar';
2708-
$rootScope.bar = 'bar';
2709-
$rootScope.$digest();
2710-
expect($rootScope.$$watchers.length).toBe(0);
2711-
expect(log.empty()).toEqual([{foo: 'foobar', bar: 'bar'}]);
2712-
2713-
$rootScope.foo = 'baz';
2714-
$rootScope.$digest();
2715-
expect($rootScope.$$watchers.length).toBe(0);
2716-
expect(log.empty()).toEqual([]);
2717-
}));
2718-
2719-
it('should only become stable when all the elements of an array have defined values', inject(function($parse, $rootScope, log) {
2720-
var fn = $parse('::[foo,bar]');
2721-
$rootScope.$watch(fn, function(value) { log(value); }, true);
2722-
2723-
expect(log.empty()).toEqual([]);
2724-
expect($rootScope.$$watchers.length).toBe(1);
2725-
2726-
$rootScope.$digest();
2727-
expect($rootScope.$$watchers.length).toBe(1);
2728-
expect(log.empty()).toEqual([[undefined, undefined]]);
2729-
2730-
$rootScope.foo = 'foo';
2731-
$rootScope.$digest();
2732-
expect($rootScope.$$watchers.length).toBe(1);
2733-
expect(log.empty()).toEqual([['foo', undefined]]);
2734-
2735-
$rootScope.foo = 'foobar';
2736-
$rootScope.bar = 'bar';
2737-
$rootScope.$digest();
2738-
expect($rootScope.$$watchers.length).toBe(0);
2739-
expect(log.empty()).toEqual([['foobar', 'bar']]);
2740-
2741-
$rootScope.foo = 'baz';
2742-
$rootScope.$digest();
2743-
expect($rootScope.$$watchers.length).toBe(0);
2744-
expect(log.empty()).toEqual([]);
2745-
}));
2746-
2747-
it('should only become stable when all the elements of an array have defined values at the end of a $digest', inject(function($parse, $rootScope, log) {
2748-
var fn = $parse('::[foo]');
2749-
$rootScope.$watch(fn, function(value) { log(value); }, true);
2750-
$rootScope.$watch('foo', function() { if ($rootScope.foo === 'bar') {$rootScope.foo = undefined; } });
2751-
2752-
$rootScope.foo = 'bar';
2753-
$rootScope.$digest();
2754-
expect($rootScope.$$watchers.length).toBe(2);
2755-
expect(log.empty()).toEqual([['bar'], [undefined]]);
2756-
2757-
$rootScope.foo = 'baz';
2758-
$rootScope.$digest();
2759-
expect($rootScope.$$watchers.length).toBe(1);
2760-
expect(log.empty()).toEqual([['baz']]);
2761-
2762-
$rootScope.bar = 'qux';
2763-
$rootScope.$digest();
2764-
expect($rootScope.$$watchers.length).toBe(1);
2765-
expect(log).toEqual([]);
2766-
}));
2691+
[true, false].forEach(function(isDeep) {
2692+
describe(isDeep ? 'deepWatch' : 'watch', function() {
2693+
it('should only become stable when all the properties of an object have defined values', inject(function($parse, $rootScope, log) {
2694+
var fn = $parse('::{foo: foo, bar: bar}');
2695+
$rootScope.$watch(fn, function(value) { log(value); }, isDeep);
2696+
2697+
expect(log.empty()).toEqual([]);
2698+
expect($rootScope.$$watchers.length).toBe(1);
2699+
2700+
$rootScope.$digest();
2701+
expect($rootScope.$$watchers.length).toBe(1);
2702+
expect(log.empty()).toEqual([{foo: undefined, bar: undefined}]);
2703+
2704+
$rootScope.foo = 'foo';
2705+
$rootScope.$digest();
2706+
expect($rootScope.$$watchers.length).toBe(1);
2707+
expect(log.empty()).toEqual([{foo: 'foo', bar: undefined}]);
2708+
2709+
$rootScope.foo = 'foobar';
2710+
$rootScope.bar = 'bar';
2711+
$rootScope.$digest();
2712+
expect($rootScope.$$watchers.length).toBe(0);
2713+
expect(log.empty()).toEqual([{foo: 'foobar', bar: 'bar'}]);
2714+
2715+
$rootScope.foo = 'baz';
2716+
$rootScope.$digest();
2717+
expect($rootScope.$$watchers.length).toBe(0);
2718+
expect(log.empty()).toEqual([]);
2719+
}));
2720+
2721+
it('should only become stable when all the elements of an array have defined values', inject(function($parse, $rootScope, log) {
2722+
var fn = $parse('::[foo,bar]');
2723+
$rootScope.$watch(fn, function(value) { log(value); }, isDeep);
2724+
2725+
expect(log.empty()).toEqual([]);
2726+
expect($rootScope.$$watchers.length).toBe(1);
2727+
2728+
$rootScope.$digest();
2729+
expect($rootScope.$$watchers.length).toBe(1);
2730+
expect(log.empty()).toEqual([[undefined, undefined]]);
2731+
2732+
$rootScope.foo = 'foo';
2733+
$rootScope.$digest();
2734+
expect($rootScope.$$watchers.length).toBe(1);
2735+
expect(log.empty()).toEqual([['foo', undefined]]);
2736+
2737+
$rootScope.foo = 'foobar';
2738+
$rootScope.bar = 'bar';
2739+
$rootScope.$digest();
2740+
expect($rootScope.$$watchers.length).toBe(0);
2741+
expect(log.empty()).toEqual([['foobar', 'bar']]);
2742+
2743+
$rootScope.foo = 'baz';
2744+
$rootScope.$digest();
2745+
expect($rootScope.$$watchers.length).toBe(0);
2746+
expect(log.empty()).toEqual([]);
2747+
}));
2748+
2749+
it('should only become stable when all the elements of an array have defined values at the end of a $digest', inject(function($parse, $rootScope, log) {
2750+
var fn = $parse('::[foo]');
2751+
$rootScope.$watch(fn, function(value) { log(value); }, isDeep);
2752+
$rootScope.$watch('foo', function() { if ($rootScope.foo === 'bar') {$rootScope.foo = undefined; } });
2753+
2754+
$rootScope.foo = 'bar';
2755+
$rootScope.$digest();
2756+
expect($rootScope.$$watchers.length).toBe(2);
2757+
expect(log.empty()).toEqual([['bar'], [undefined]]);
2758+
2759+
$rootScope.foo = 'baz';
2760+
$rootScope.$digest();
2761+
expect($rootScope.$$watchers.length).toBe(1);
2762+
expect(log.empty()).toEqual([['baz']]);
2763+
2764+
$rootScope.bar = 'qux';
2765+
$rootScope.$digest();
2766+
expect($rootScope.$$watchers.length).toBe(1);
2767+
expect(log).toEqual([]);
2768+
}));
2769+
});
2770+
});
27672771
});
27682772
});
27692773

0 commit comments

Comments
 (0)