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

Commit 775281c

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 f1d0f03 commit 775281c

File tree

3 files changed

+107
-158
lines changed

3 files changed

+107
-158
lines changed

‎src/ng/directive/ngClass.js

+1-44
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,14 +90,6 @@ 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);
105-
}
106-
}
107-
10893
function ngClassWatchAction(newClassString) {
10994
if (oldModulo === selector) {
11095
updateClasses(oldClassString, newClassString);
@@ -152,34 +137,6 @@ function classDirective(name, selector) {
152137

153138
return classString;
154139
}
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-
}
183140
}
184141

185142
/**

‎src/ng/parse.js

+26-38
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,22 +1907,28 @@ 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;
19451933
if (watchDelegate && watchDelegate !== inputsWatchDelegate) {
19461934
fn.$$watchDelegate = watchDelegate;

‎test/ng/parseSpec.js

+80-76
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)