fix(ngOptions): do not watch properties starting with $ #12010
Conversation
Expressions that compute labels and track by values for ngOptions were being called for properties, that start with $ even though those properties were being ignored as options. Closes angular#11930
| @@ -303,6 +303,7 @@ var ngOptionsDirective = ['$compile', '$parse', function($compile, $parse) { | |||
| values = values || []; | |||
|
|
|||
| Object.keys(values).forEach(function getWatchable(key) { | |||
| if (key.charAt(0) === '$') return; | |||
There was a problem hiding this comment.
Do we really want to ignore $ (and not just $$) properties ?
$$ are (by convention) private, but $ are (by convention again) Angular-specific, but not private.
There was a problem hiding this comment.
@gkalpak - good question. Right now we are ignoring such properties as options, so there is a precedent: https://github.com/angular/angular.js/blob/master/test/ng/directive/ngOptionsSpec.js#L434-L448
Also angular.equals and ngRepeat ignore single $ too.
There was a problem hiding this comment.
Good point. For some reason, ToJsonReplacer() was modifed some time ago to only ignore $$ properties (but everything else (e.g. ngRepeat, ngOptions, equals) kept ignoring $ as well. Too much breaking change I guess :)
So, it's fine as it is 😃
|
LGTM |
It turns out that the options that are displayed are more constrained than just whether the property starts with a $ character. If the values collection is array-like then we only display the options that are identified by numerical properties - it's an array right? So this commit aligns `getWatchables` with `getOptions`. See #12010
Expressions that compute labels and track by values for ngOptions were being called for properties, that start with $ even though those properties were being ignored as options. Closes angular#11930 Closes angular#12010
It turns out that the options that are displayed are more constrained than just whether the property starts with a $ character. If the values collection is array-like then we only display the options that are identified by numerical properties - it's an array right? So this commit aligns `getWatchables` with `getOptions`. See angular#12010
Expressions that compute labels and track by values for ngOptions were
being called for properties, that start with $ even though those properties
were being ignored as options.
Closes #11930