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

ngOptions incorrectly evaluates label for $$hashKey when watching labels #11930

Closed
ryanhart2 opened this issue May 22, 2015 · 2 comments
Closed

Comments

@ryanhart2
Copy link
Contributor

Overview of the Issue
Within a directive, there is a variable on the scope which is an array of arrays. The directive template uses ng-repeat to repeat a <select> for each of the "child" arrays in this array variable. Each <select> has an ng-options attribute with the format "item as translate(item) for item in array" where array is provided by the ng-repeat and translate is a function on the scope that translates the item value for display.
At this point the array provided by ng-repeat has a $$hashkey property. This property is correctly omitted from the options on the <select>, however, the translate function is invoked for this property in addition to the other items in the array, which is not expected.

Motivation for or Use Case
The motivation is to have a directive that can be provided with an array of selections that the user is required to make, with each selection variable being an array of the available options. The directive will then display a select element for each selection containing the relevant options. The directive accepts an optional translate-options attribute which, when set to true, will result in the options being translated. The translation should only affect the display of the options and not the model value.

Angular Version(s)
This issue does not occur with version 1.3.15
This issue does occur with versions 1.4.0-beta.0 through to at least 1.4.0-rc.2

Browsers and Operating System
Chrome 43.0.2357.65 m, Windows 8.1

Reproduce the Error
http://plnkr.co/edit/nyOOCbqcz3QlPvjddW3R?p=preview

Related Issues
None

Suggest a Fix
When Object.keys(values).forEach is executed below, values has a $$hashKey property which is then passed in to the getWatchable function. This results in displayFn being invoked with the locals parameter being an object with a value similar to {value: "object:50"}. A potential fix is for the getWatchable function to ignore keys that begin with '$$'.

getWatchables: $parse(valuesFn, function(values) {
  // Create a collection of things that we would like to watch (watchedArray)
  // so that they can all be watched using a single $watchCollection
  // that only runs the handler once if anything changes
  var watchedArray = [];
  values = values || [];

  Object.keys(values).forEach(function getWatchable(key) {
    var locals = getLocals(values[key], key);
    var selectValue = getTrackByValueFn(values[key], locals);
    watchedArray.push(selectValue);

    // Only need to watch the displayFn if there is a specific label expression
    if (match[2] || match[1]) {
      var label = displayFn(scope, locals);
      watchedArray.push(label);
    }

    // Only need to watch the disableWhenFn if there is a specific disable expression
    if (match[4]) {
      var disableWhen = disableWhenFn(scope, locals);
      watchedArray.push(disableWhen);
    }
  });
  return watchedArray;
})
@pkozlowski-opensource
Copy link
Member

Wow, I love bug reports like this one! Very detailed and to the point. Thnx so much @ryanhart2 !!!

@gkalpak
Copy link
Member

gkalpak commented May 22, 2015

What he said !

angular.forEach() knew to ignore $$ properties I guess, Object.keys().forEach() doesn't.
Seems like we need a new angular.forEachKey() :)

@Narretz Narretz added this to the 1.4.x - jaracimrman-existence milestone May 25, 2015
Narretz added a commit to Narretz/angular.js that referenced this issue May 28, 2015
Narretz added a commit to Narretz/angular.js that referenced this issue May 28, 2015
@petebacondarwin petebacondarwin self-assigned this Jun 1, 2015
@petebacondarwin petebacondarwin modified the milestones: 1.4.1, 1.4.x Jun 1, 2015
petebacondarwin added a commit to petebacondarwin/angular.js that referenced this issue Jun 2, 2015
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
netman92 pushed a commit to netman92/angular.js that referenced this issue Aug 8, 2015
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
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants