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

Commit a0bb123

Browse files
jbedardjbedard
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.
1 parent 0d9d57d commit a0bb123

File tree

3 files changed

+110
-160
lines changed

3 files changed

+110
-160
lines changed

‎src/ng/directive/ngClass.js

+2-43
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);
@@ -97,15 +90,9 @@ function classDirective(name, selector) {
9790
oldModulo = newModulo;
9891
}
9992

100-
function ngClassOneTimeWatchAction(newClassValue) {
93+
function ngClassWatchAction(newClassValue) {
10194
var newClassString = toClassString(newClassValue);
10295

103-
if (newClassString !== oldClassString) {
104-
ngClassWatchAction(newClassString);
105-
}
106-
}
107-
108-
function ngClassWatchAction(newClassString) {
10996
if (oldModulo === selector) {
11097
updateClasses(oldClassString, newClassString);
11198
}
@@ -152,34 +139,6 @@ function classDirective(name, selector) {
152139

153140
return classString;
154141
}
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-
}
183142
}
184143

185144
/**

‎src/ng/parse.js

+28-41
Original file line numberDiff line numberDiff line change
@@ -1772,8 +1772,8 @@ function $ParseProvider() {
17721772
if (parsedExpression.constant) {
17731773
parsedExpression.$$watchDelegate = constantWatchDelegate;
17741774
} else if (oneTime) {
1775-
parsedExpression.$$watchDelegate = parsedExpression.literal ?
1776-
oneTimeLiteralWatchDelegate : oneTimeWatchDelegate;
1775+
parsedExpression.oneTime = true;
1776+
parsedExpression.$$watchDelegate = oneTimeWatchDelegate;
17771777
} else if (parsedExpression.inputs) {
17781778
parsedExpression.$$watchDelegate = inputsWatchDelegate;
17791779
}
@@ -1859,6 +1859,7 @@ function $ParseProvider() {
18591859
}
18601860

18611861
function oneTimeWatchDelegate(scope, listener, objectEquality, parsedExpression, prettyPrintExpression) {
1862+
var doneCriteria = parsedExpression.literal ? isAllDefined : isDefined;
18621863
var unwatch, lastValue;
18631864
if (parsedExpression.inputs) {
18641865
unwatch = inputsWatchDelegate(scope, oneTimeListener, objectEquality, parsedExpression, prettyPrintExpression);
@@ -1875,41 +1876,22 @@ function $ParseProvider() {
18751876
if (isFunction(listener)) {
18761877
listener(value, old, scope);
18771878
}
1878-
if (isDefined(value)) {
1879+
if (doneCriteria(value)) {
18791880
scope.$$postDigest(function() {
1880-
if (isDefined(lastValue)) {
1881+
if (doneCriteria(lastValue)) {
18811882
unwatch();
18821883
}
18831884
});
18841885
}
18851886
}
18861887
}
18871888

1888-
function oneTimeLiteralWatchDelegate(scope, listener, objectEquality, parsedExpression) {
1889-
var unwatch, lastValue;
1890-
unwatch = scope.$watch(function oneTimeWatch(scope) {
1891-
return parsedExpression(scope);
1892-
}, function oneTimeListener(value, old, scope) {
1893-
lastValue = value;
1894-
if (isFunction(listener)) {
1895-
listener(value, old, scope);
1896-
}
1897-
if (isAllDefined(value)) {
1898-
scope.$$postDigest(function() {
1899-
if (isAllDefined(lastValue)) unwatch();
1900-
});
1901-
}
1902-
}, objectEquality);
1903-
1904-
return unwatch;
1905-
1906-
function isAllDefined(value) {
1907-
var allDefined = true;
1908-
forEach(value, function(val) {
1909-
if (!isDefined(val)) allDefined = false;
1910-
});
1911-
return allDefined;
1912-
}
1889+
function isAllDefined(value) {
1890+
var allDefined = true;
1891+
forEach(value, function(val) {
1892+
if (!isDefined(val)) allDefined = false;
1893+
});
1894+
return allDefined;
19131895
}
19141896

19151897
function constantWatchDelegate(scope, listener, objectEquality, parsedExpression) {
@@ -1925,26 +1907,31 @@ function $ParseProvider() {
19251907
var watchDelegate = parsedExpression.$$watchDelegate;
19261908
var useInputs = false;
19271909

1928-
var regularWatch =
1929-
watchDelegate !== oneTimeLiteralWatchDelegate &&
1930-
watchDelegate !== oneTimeWatchDelegate;
1910+
var doneCriteria = parsedExpression.literal ? isAllDefined : isDefined;
19311911

1932-
var fn = regularWatch ? function regularInterceptedExpression(scope, locals, assign, inputs) {
1912+
function regularInterceptedExpression(scope, locals, assign, inputs) {
19331913
var value = useInputs && inputs ? inputs[0] : parsedExpression(scope, locals, assign, inputs);
19341914
return interceptorFn(value, scope, locals);
1935-
} : function oneTimeInterceptedExpression(scope, locals, assign, inputs) {
1936-
var value = parsedExpression(scope, locals, assign, inputs);
1915+
}
1916+
1917+
function oneTimeInterceptedExpression(scope, locals, assign, inputs) {
1918+
var value = useInputs && inputs ? inputs[0] : parsedExpression(scope, locals, assign, inputs);
19371919
var result = interceptorFn(value, scope, locals);
19381920
// we only return the interceptor's result if the
19391921
// initial value is defined (for bind-once)
1940-
return isDefined(value) ? result : value;
1941-
};
1922+
return doneCriteria(value) ? result : value;
1923+
}
1924+
1925+
var fn = parsedExpression.oneTime ? oneTimeInterceptedExpression : regularInterceptedExpression;
1926+
1927+
// Propogate the literal/oneTime attributes
1928+
fn.literal = parsedExpression.literal;
1929+
fn.oneTime = parsedExpression.oneTime;
19421930

1943-
// Propagate $$watchDelegates other then inputsWatchDelegate
1931+
// Propagate or create inputs / $$watchDelegates
19441932
useInputs = !parsedExpression.inputs;
1945-
if (parsedExpression.$$watchDelegate &&
1946-
parsedExpression.$$watchDelegate !== inputsWatchDelegate) {
1947-
fn.$$watchDelegate = parsedExpression.$$watchDelegate;
1933+
if (watchDelegate && watchDelegate !== inputsWatchDelegate) {
1934+
fn.$$watchDelegate = watchDelegate;
19481935
fn.inputs = parsedExpression.inputs;
19491936
} else if (!interceptorFn.$stateful) {
19501937
// If there is an interceptor, but no watchDelegate then treat the interceptor like

‎test/ng/parseSpec.js

+80-76
Original file line numberDiff line numberDiff line change
@@ -2686,82 +2686,86 @@ describe('parser', function() {
26862686
expect($parse(':: ').literal).toBe(true);
26872687
}));
26882688

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

0 commit comments

Comments
 (0)