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

Commit 14638f4

Browse files
fix(ngOptions): only watch numeric properties of an array
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
1 parent 0c1fbdd commit 14638f4

File tree

2 files changed

+33
-24
lines changed

2 files changed

+33
-24
lines changed

src/ng/directive/ngOptions.js

+29-22
Original file line numberDiff line numberDiff line change
@@ -292,20 +292,41 @@ var ngOptionsDirective = ['$compile', '$parse', function($compile, $parse) {
292292
this.disabled = disabled;
293293
}
294294

295+
function getOptionValuesKeys(optionValues) {
296+
var optionValuesKeys;
297+
298+
if (!keyName && isArrayLike(optionValues)) {
299+
optionValuesKeys = optionValues;
300+
} else {
301+
// if object, extract keys, in enumeration order, unsorted
302+
optionValuesKeys = [];
303+
for (var itemKey in optionValues) {
304+
if (optionValues.hasOwnProperty(itemKey) && itemKey.charAt(0) !== '$') {
305+
optionValuesKeys.push(itemKey);
306+
}
307+
}
308+
}
309+
return optionValuesKeys;
310+
}
311+
295312
return {
296313
trackBy: trackBy,
297314
getTrackByValue: getTrackByValue,
298-
getWatchables: $parse(valuesFn, function(values) {
315+
getWatchables: $parse(valuesFn, function(optionValues) {
299316
// Create a collection of things that we would like to watch (watchedArray)
300317
// so that they can all be watched using a single $watchCollection
301318
// that only runs the handler once if anything changes
302319
var watchedArray = [];
303-
values = values || [];
320+
optionValues = optionValues || [];
304321

305-
Object.keys(values).forEach(function getWatchable(key) {
306-
if (key.charAt(0) === '$') return;
307-
var locals = getLocals(values[key], key);
308-
var selectValue = getTrackByValueFn(values[key], locals);
322+
var optionValuesKeys = getOptionValuesKeys(optionValues);
323+
var optionValuesLength = optionValuesKeys.length;
324+
for (var index = 0; index < optionValuesLength; index++) {
325+
var key = (optionValues === optionValuesKeys) ? index : optionValuesKeys[index];
326+
var value = optionValues[key];
327+
328+
var locals = getLocals(optionValues[key], key);
329+
var selectValue = getTrackByValueFn(optionValues[key], locals);
309330
watchedArray.push(selectValue);
310331

311332
// Only need to watch the displayFn if there is a specific label expression
@@ -319,7 +340,7 @@ var ngOptionsDirective = ['$compile', '$parse', function($compile, $parse) {
319340
var disableWhen = disableWhenFn(scope, locals);
320341
watchedArray.push(disableWhen);
321342
}
322-
});
343+
}
323344
return watchedArray;
324345
}),
325346

@@ -331,21 +352,7 @@ var ngOptionsDirective = ['$compile', '$parse', function($compile, $parse) {
331352
// The option values were already computed in the `getWatchables` fn,
332353
// which must have been called to trigger `getOptions`
333354
var optionValues = valuesFn(scope) || [];
334-
var optionValuesKeys;
335-
336-
337-
if (!keyName && isArrayLike(optionValues)) {
338-
optionValuesKeys = optionValues;
339-
} else {
340-
// if object, extract keys, in enumeration order, unsorted
341-
optionValuesKeys = [];
342-
for (var itemKey in optionValues) {
343-
if (optionValues.hasOwnProperty(itemKey) && itemKey.charAt(0) !== '$') {
344-
optionValuesKeys.push(itemKey);
345-
}
346-
}
347-
}
348-
355+
var optionValuesKeys = getOptionValuesKeys(optionValues);
349356
var optionValuesLength = optionValuesKeys.length;
350357

351358
for (var index = 0; index < optionValuesLength; index++) {

test/ng/directive/ngOptionsSpec.js

+4-2
Original file line numberDiff line numberDiff line change
@@ -448,7 +448,7 @@ describe('ngOptions', function() {
448448
});
449449

450450

451-
it('should not watch array properties that start with $ or $$', function() {
451+
it('should not watch non-numeric array properties', function() {
452452
createSelect({
453453
'ng-options': 'value as createLabel(value) for value in array',
454454
'ng-model': 'selected'
@@ -457,13 +457,16 @@ describe('ngOptions', function() {
457457
scope.array = ['a', 'b', 'c'];
458458
scope.array.$$private = 'do not watch';
459459
scope.array.$property = 'do not watch';
460+
scope.array.other = 'do not watch';
461+
scope.array.fn = function() {};
460462
scope.selected = 'b';
461463
scope.$digest();
462464

463465
expect(scope.createLabel).toHaveBeenCalledWith('a');
464466
expect(scope.createLabel).toHaveBeenCalledWith('b');
465467
expect(scope.createLabel).toHaveBeenCalledWith('c');
466468
expect(scope.createLabel).not.toHaveBeenCalledWith('do not watch');
469+
expect(scope.createLabel).not.toHaveBeenCalledWith(jasmine.any(Function));
467470
});
468471

469472

@@ -482,7 +485,6 @@ describe('ngOptions', function() {
482485
expect(scope.createLabel).not.toHaveBeenCalledWith('$property');
483486
});
484487

485-
486488
it('should allow expressions over multiple lines', function() {
487489
scope.isNotFoo = function(item) {
488490
return item.name !== 'Foo';

0 commit comments

Comments
 (0)