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 #15858
This commit is contained in:
Jason Bedard
2017-03-31 00:36:19 -07:00
committed by Martin Staffa
parent 93879b3c72
commit 189461f9bf
3 changed files with 100 additions and 145 deletions
+9 -46
View File
@@ -14,13 +14,6 @@ function classDirective(name, selector) {
return {
restrict: 'AC',
link: function(scope, element, attr) {
var expression = attr[name].trim();
var isOneTime = (expression.charAt(0) === ':') && (expression.charAt(1) === ':');
var watchInterceptor = isOneTime ? toFlatValue : toClassString;
var watchExpression = $parse(expression, watchInterceptor);
var watchAction = isOneTime ? ngClassOneTimeWatchAction : ngClassWatchAction;
var classCounts = element.data('$classCounts');
var oldModulo = true;
var oldClassString;
@@ -43,7 +36,7 @@ function classDirective(name, selector) {
scope.$watch(indexWatchExpression, ngClassIndexWatchAction);
}
scope.$watch(watchExpression, watchAction, isOneTime);
scope.$watch($parse(attr[name], toClassString), ngClassWatchAction);
function addClasses(classString) {
classString = digestClassCounts(split(classString), 1);
@@ -85,9 +78,9 @@ function classDirective(name, selector) {
}
function ngClassIndexWatchAction(newModulo) {
// This watch-action should run before the `ngClass[OneTime]WatchAction()`, thus it
// This watch-action should run before the `ngClassWatchAction()`, thus it
// adds/removes `oldClassString`. If the `ngClass` expression has changed as well, the
// `ngClass[OneTime]WatchAction()` will update the classes.
// `ngClassWatchAction()` will update the classes.
if (newModulo === selector) {
addClasses(oldClassString);
} else {
@@ -97,15 +90,13 @@ function classDirective(name, selector) {
oldModulo = newModulo;
}
function ngClassOneTimeWatchAction(newClassValue) {
var newClassString = toClassString(newClassValue);
if (newClassString !== oldClassString) {
ngClassWatchAction(newClassString);
}
}
function ngClassWatchAction(newClassString) {
// When using a one-time binding the newClassString will return
// the pre-interceptor value until the one-time is complete
if (!isString(newClassString)) {
newClassString = toClassString(newClassString);
}
if (oldModulo === selector) {
updateClasses(oldClassString, newClassString);
}
@@ -152,34 +143,6 @@ function classDirective(name, selector) {
return classString;
}
function toFlatValue(classValue) {
var flatValue = classValue;
if (isArray(classValue)) {
flatValue = classValue.map(toFlatValue);
} else if (isObject(classValue)) {
var hasUndefined = false;
flatValue = Object.keys(classValue).filter(function(key) {
var value = classValue[key];
if (!hasUndefined && isUndefined(value)) {
hasUndefined = true;
}
return value;
});
if (hasUndefined) {
// Prevent the `oneTimeLiteralWatchInterceptor` from unregistering
// the watcher, by including at least one `undefined` value.
flatValue.push(undefined);
}
}
return flatValue;
}
}
/**
+26 -38
View File
@@ -1765,8 +1765,8 @@ function $ParseProvider() {
if (parsedExpression.constant) {
parsedExpression.$$watchDelegate = constantWatchDelegate;
} else if (oneTime) {
parsedExpression.$$watchDelegate = parsedExpression.literal ?
oneTimeLiteralWatchDelegate : oneTimeWatchDelegate;
parsedExpression.oneTime = true;
parsedExpression.$$watchDelegate = oneTimeWatchDelegate;
} else if (parsedExpression.inputs) {
parsedExpression.$$watchDelegate = inputsWatchDelegate;
}
@@ -1852,6 +1852,7 @@ function $ParseProvider() {
}
function oneTimeWatchDelegate(scope, listener, objectEquality, parsedExpression, prettyPrintExpression) {
var isDone = parsedExpression.literal ? isAllDefined : isDefined;
var unwatch, lastValue;
if (parsedExpression.inputs) {
unwatch = inputsWatchDelegate(scope, oneTimeListener, objectEquality, parsedExpression, prettyPrintExpression);
@@ -1868,9 +1869,9 @@ function $ParseProvider() {
if (isFunction(listener)) {
listener(value, old, scope);
}
if (isDefined(value)) {
if (isDone(value)) {
scope.$$postDigest(function() {
if (isDefined(lastValue)) {
if (isDone(lastValue)) {
unwatch();
}
});
@@ -1878,31 +1879,12 @@ function $ParseProvider() {
}
}
function oneTimeLiteralWatchDelegate(scope, listener, objectEquality, parsedExpression) {
var unwatch, lastValue;
unwatch = scope.$watch(function oneTimeWatch(scope) {
return parsedExpression(scope);
}, function oneTimeListener(value, old, scope) {
lastValue = value;
if (isFunction(listener)) {
listener(value, old, scope);
}
if (isAllDefined(value)) {
scope.$$postDigest(function() {
if (isAllDefined(lastValue)) unwatch();
});
}
}, objectEquality);
return unwatch;
function isAllDefined(value) {
var allDefined = true;
forEach(value, function(val) {
if (!isDefined(val)) allDefined = false;
});
return allDefined;
}
function isAllDefined(value) {
var allDefined = true;
forEach(value, function(val) {
if (!isDefined(val)) allDefined = false;
});
return allDefined;
}
function constantWatchDelegate(scope, listener, objectEquality, parsedExpression) {
@@ -1918,22 +1900,28 @@ function $ParseProvider() {
var watchDelegate = parsedExpression.$$watchDelegate;
var useInputs = false;
var regularWatch =
watchDelegate !== oneTimeLiteralWatchDelegate &&
watchDelegate !== oneTimeWatchDelegate;
var isDone = parsedExpression.literal ? isAllDefined : isDefined;
var fn = regularWatch ? function regularInterceptedExpression(scope, locals, assign, inputs) {
function regularInterceptedExpression(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);
}
function oneTimeInterceptedExpression(scope, locals, assign, inputs) {
var value = useInputs && inputs ? inputs[0] : parsedExpression(scope, locals, assign, inputs);
var result = interceptorFn(value, scope, locals);
// we only return the interceptor's result if the
// initial value is defined (for bind-once)
return isDefined(value) ? result : value;
};
return isDone(value) ? result : value;
}
// Propagate $$watchDelegates other then inputsWatchDelegate
var fn = parsedExpression.oneTime ? oneTimeInterceptedExpression : regularInterceptedExpression;
// Propogate the literal/oneTime attributes
fn.literal = parsedExpression.literal;
fn.oneTime = parsedExpression.oneTime;
// Propagate or create inputs / $$watchDelegates
useInputs = !parsedExpression.inputs;
if (watchDelegate && watchDelegate !== inputsWatchDelegate) {
fn.$$watchDelegate = watchDelegate;
+65 -61
View File
@@ -2688,82 +2688,86 @@ describe('parser', function() {
expect($parse(':: ').literal).toBe(true);
}));
it('should only become stable when all the properties of an object have defined values', inject(function($parse, $rootScope, log) {
var fn = $parse('::{foo: foo, bar: bar}');
$rootScope.$watch(fn, function(value) { log(value); }, true);
[true, false].forEach(function(isDeep) {
describe(isDeep ? 'deepWatch' : 'watch', function() {
it('should only become stable when all the properties of an object have defined values', inject(function($parse, $rootScope, log) {
var fn = $parse('::{foo: foo, bar: bar}');
$rootScope.$watch(fn, function(value) { log(value); }, isDeep);
expect(log.empty()).toEqual([]);
expect($rootScope.$$watchers.length).toBe(1);
expect(log.empty()).toEqual([]);
expect($rootScope.$$watchers.length).toBe(1);
$rootScope.$digest();
expect($rootScope.$$watchers.length).toBe(1);
expect(log.empty()).toEqual([{foo: undefined, bar: undefined}]);
$rootScope.$digest();
expect($rootScope.$$watchers.length).toBe(1);
expect(log.empty()).toEqual([{foo: undefined, bar: undefined}]);
$rootScope.foo = 'foo';
$rootScope.$digest();
expect($rootScope.$$watchers.length).toBe(1);
expect(log.empty()).toEqual([{foo: 'foo', bar: undefined}]);
$rootScope.foo = 'foo';
$rootScope.$digest();
expect($rootScope.$$watchers.length).toBe(1);
expect(log.empty()).toEqual([{foo: 'foo', bar: undefined}]);
$rootScope.foo = 'foobar';
$rootScope.bar = 'bar';
$rootScope.$digest();
expect($rootScope.$$watchers.length).toBe(0);
expect(log.empty()).toEqual([{foo: 'foobar', bar: 'bar'}]);
$rootScope.foo = 'foobar';
$rootScope.bar = 'bar';
$rootScope.$digest();
expect($rootScope.$$watchers.length).toBe(0);
expect(log.empty()).toEqual([{foo: 'foobar', bar: 'bar'}]);
$rootScope.foo = 'baz';
$rootScope.$digest();
expect($rootScope.$$watchers.length).toBe(0);
expect(log.empty()).toEqual([]);
}));
$rootScope.foo = 'baz';
$rootScope.$digest();
expect($rootScope.$$watchers.length).toBe(0);
expect(log.empty()).toEqual([]);
}));
it('should only become stable when all the elements of an array have defined values', inject(function($parse, $rootScope, log) {
var fn = $parse('::[foo,bar]');
$rootScope.$watch(fn, function(value) { log(value); }, true);
it('should only become stable when all the elements of an array have defined values', inject(function($parse, $rootScope, log) {
var fn = $parse('::[foo,bar]');
$rootScope.$watch(fn, function(value) { log(value); }, isDeep);
expect(log.empty()).toEqual([]);
expect($rootScope.$$watchers.length).toBe(1);
expect(log.empty()).toEqual([]);
expect($rootScope.$$watchers.length).toBe(1);
$rootScope.$digest();
expect($rootScope.$$watchers.length).toBe(1);
expect(log.empty()).toEqual([[undefined, undefined]]);
$rootScope.$digest();
expect($rootScope.$$watchers.length).toBe(1);
expect(log.empty()).toEqual([[undefined, undefined]]);
$rootScope.foo = 'foo';
$rootScope.$digest();
expect($rootScope.$$watchers.length).toBe(1);
expect(log.empty()).toEqual([['foo', undefined]]);
$rootScope.foo = 'foo';
$rootScope.$digest();
expect($rootScope.$$watchers.length).toBe(1);
expect(log.empty()).toEqual([['foo', undefined]]);
$rootScope.foo = 'foobar';
$rootScope.bar = 'bar';
$rootScope.$digest();
expect($rootScope.$$watchers.length).toBe(0);
expect(log.empty()).toEqual([['foobar', 'bar']]);
$rootScope.foo = 'foobar';
$rootScope.bar = 'bar';
$rootScope.$digest();
expect($rootScope.$$watchers.length).toBe(0);
expect(log.empty()).toEqual([['foobar', 'bar']]);
$rootScope.foo = 'baz';
$rootScope.$digest();
expect($rootScope.$$watchers.length).toBe(0);
expect(log.empty()).toEqual([]);
}));
$rootScope.foo = 'baz';
$rootScope.$digest();
expect($rootScope.$$watchers.length).toBe(0);
expect(log.empty()).toEqual([]);
}));
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) {
var fn = $parse('::[foo]');
$rootScope.$watch(fn, function(value) { log(value); }, true);
$rootScope.$watch('foo', function() { if ($rootScope.foo === 'bar') {$rootScope.foo = undefined; } });
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) {
var fn = $parse('::[foo]');
$rootScope.$watch(fn, function(value) { log(value); }, isDeep);
$rootScope.$watch('foo', function() { if ($rootScope.foo === 'bar') {$rootScope.foo = undefined; } });
$rootScope.foo = 'bar';
$rootScope.$digest();
expect($rootScope.$$watchers.length).toBe(2);
expect(log.empty()).toEqual([['bar'], [undefined]]);
$rootScope.foo = 'bar';
$rootScope.$digest();
expect($rootScope.$$watchers.length).toBe(2);
expect(log.empty()).toEqual([['bar'], [undefined]]);
$rootScope.foo = 'baz';
$rootScope.$digest();
expect($rootScope.$$watchers.length).toBe(1);
expect(log.empty()).toEqual([['baz']]);
$rootScope.foo = 'baz';
$rootScope.$digest();
expect($rootScope.$$watchers.length).toBe(1);
expect(log.empty()).toEqual([['baz']]);
$rootScope.bar = 'qux';
$rootScope.$digest();
expect($rootScope.$$watchers.length).toBe(1);
expect(log).toEqual([]);
}));
$rootScope.bar = 'qux';
$rootScope.$digest();
expect($rootScope.$$watchers.length).toBe(1);
expect(log).toEqual([]);
}));
});
});
});
});