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

Hard coded <option> causes error "TypeError: Cannot read property 'getOptionFromViewValue' of undefined" #11685

Closed
godza opened this issue Apr 23, 2015 · 13 comments · Fixed by #13012

Comments

@godza
Copy link

godza commented Apr 23, 2015

Hi, in documentation is stated that 'empty option' can be used when using ng-options. Here is direct quote

Optionally a single hard-coded option element, with the value set to an empty string, can be nested into the select element. This element will then represent the null or "not selected" option

I used that in 1.3 very heavily and it worked flawlessly, but in 1.4 i'm getting error

TypeError: Cannot read property 'getOptionFromViewValue' of undefined

Just to be clear, here is the template i'm using

<select ng-options="option.id as option.name for option in selectable_options">
  <option value="">Choose an option</option>
</select>

Is this planed regression or a bug? If this is planed regression, is there a workaround for the thing i'm trying to accomplish?

@gkalpak
Copy link
Member

gkalpak commented Apr 23, 2015

I can't reproduce the error. Which version are you using. Ideally you should provide and live reproduction using CodePen, JSFiddle, Plnkr or a similar.

You should also use ngModel on the select.

@godza
Copy link
Author

godza commented Apr 23, 2015

Hi, i was using 1.4 rc and i built latest angular from git master, but it's the same error. Here is the plunkr but error thrown there is not the one you get when you're running from files on your computer

As you can see, the select is not populated with options.

http://plnkr.co/edit/l0SV6rcOeVufxJ2f6oNm?p=preview

@gkalpak
Copy link
Member

gkalpak commented Apr 23, 2015

I don't get any error. (Neither on my computer nor in the plnkr.)
ngOptions requires ngModel, so it won't populate your <select>, unless you have ngModel on it.

Updated plnkr

@lammel
Copy link

lammel commented Apr 23, 2015

We are seeing the same issue in our application. But only in a particular situation (we pass the a model to a directive which in turn displays the select with the hardcoded empty option). Other usage of select with ng-options works fine (also with the hardcoded empty option).

We try to isolate the issue in a plunker for further analysis.

@godza
Copy link
Author

godza commented Apr 23, 2015

@gkalpak i finally managed to reporoduce the problem, even on plunker.

The catch is that template of customSelect directive in my example needs to be loaded via templateUrl. In that case i see errors and list is not printed.

http://plnkr.co/edit/l0SV6rcOeVufxJ2f6oNm?p=preview
  • If you edit select_template.html file and remove empty option it works
  • if you copy/paste select_template.html content and paste it in template property of directive definition, it will work even with empty option

@bbasic
Copy link

bbasic commented Apr 23, 2015

Just to be thorough.
It will work in a situation where replace: true is not used.

plunker
Uncomment the line 27 in app.js to see the error.

@gkalpak
Copy link
Member

gkalpak commented Apr 23, 2015

Indeed it seems that the combination of templateUrl (instead of template) and replace: true results in no options being created.
I am not sure if this should be fixed though, considering that replace: true has been deprecated.

@petebacondarwin, wdyt ?

@godza
Copy link
Author

godza commented Apr 23, 2015

@gkalpak even though it's deprecated it should be working properly until removed.

@marcelgwerder
Copy link

I think I have the same issue. I have a simple select with an additional custom directive that uses

$compile(element.contents())(scope); 

It only works if I either remove the empty option:

<select ng-options="country.code as country.title for country in countries" ng-model="selectedCountry" random-directive>
</select>

Or the custom directive:

<select ng-options="country.code as country.title for country in countries" ng-model="selectedCountry">
    <option value="">Please select...</option>
</select>

But both together throw a TypeError: Cannot read property 'getOptionFromViewValue' of undefined.

http://plnkr.co/edit/kIZqA67L1adub2f179Yj?p=preview

This worked with 1.3:

http://plnkr.co/edit/VA5NeFeTYExSb7jCtOYs?p=preview

@egervari
Copy link

Thank you thank you for the replace: true post. This solved my problem as well.

@Narretz Narretz self-assigned this Sep 27, 2015
@Narretz
Copy link
Contributor

Narretz commented Sep 28, 2015

Here's why it happens:
In a regular select, the elements add themselves as options to the selectCtrl. It also adds an element $destroy listener that removes the option and forces a render.
In a ngOptions select, the first element is extracted from the element, and compiled separately. Since it's not part of the document, it doesn't have a selectCtrl parent, and it doesn't get added.
If another directive compiles the select contents before ngOptions is linked, the option gets added. But when ngOptions runs it also gets removed from the DOM, and its destroy listener is called, which calls render, which delegates to ngOptions, which at this point doesn't have options set.
So the easy fix is simply to check if options are defined here: https://github.com/angular/angular.js/blob/1.4.6/src/ng/directive/ngOptions.js#L459
But the problem also raises some questions about the interop between ngOptions and select ...

@petebacondarwin
Copy link
Contributor

A workaround is to ensure that your random directive has a higher priority than the select and ngOptions directives: http://plnkr.co/edit/kIZqA67L1adub2f179Yj?p=preview

Narretz added a commit to Narretz/angular.js that referenced this issue Sep 28, 2015
This can happen in the following case:
- there's a blank option inside the select
- another directive on the select element compiles the contents of it before ngOptions is linked.

Now this happens:
- the option directive is compiled and adds an element $destroy listener that calls ngModel.$render
- when ngOptions processes the blank option, it removes the element, and
triggers the $destroy listener
- ngModel.$render delegates to selectCtrl.writeValue, which accesses the options
- in that phase, the options aren't yet set

Fixes angular#11685
Narretz added a commit to Narretz/angular.js that referenced this issue Oct 1, 2015
When ngOptions is present on a select, the option directive should not be able to add
options to the selectCtrl. This will cause errors during the ngOptions lifecycle.

This can happen in the following case:
- there's a blank option inside the select
- another directive on the select element compiles the contents of it before ngOptions is linked.

Now this happens:
- the option directive is compiled and adds an element $destroy listener that calls ngModel.$render
- when ngOptions processes the blank option, it removes the element, and
triggers the $destroy listener
- ngModel.$render delegates to selectCtrl.writeValue, which accesses the options
- in that phase, the options aren't yet set

Fixes angular#11685
Narretz added a commit to Narretz/angular.js that referenced this issue Oct 1, 2015
When ngOptions is present on a select, the option directive should not be able to add
options to the selectCtrl. This will cause errors during the ngOptions lifecycle.

This can happen in the following case:
- there's a blank option inside the select
- another directive on the select element compiles the contents of it before ngOptions is linked.

Now this happens:
- the option directive is compiled and adds an element $destroy listener that calls ngModel.$render
- when ngOptions processes the blank option, it removes the element, and
triggers the $destroy listener
- ngModel.$render delegates to selectCtrl.writeValue, which accesses the options
- in that phase, the options aren't yet set

Fixes angular#11685
petebacondarwin pushed a commit to petebacondarwin/angular.js that referenced this issue Oct 5, 2015
When ngOptions is present on a select, the option directive should not be able to
register options on the selectCtrl since this may cause errors during the
ngOptions lifecycle.

This can happen in the following cases:

- there is a blank option below the select element, an ngModel
directive, an ngOptions directive and some other directive on the select
element, which compiles the children of the select
(i.e. the option elements) before ngOptions is has finished linking.

- there is a blank option below the select element, an ngModel
directive, an ngOptions directive and another directive, which uses
templateUrl and replace:true.

What happens is:
- the option directive is compiled and adds an element `$destroy` listener
that will call `ngModel.$render` when the option element is removed.
- when `ngOptions` processes the option, it removes the element, and
triggers the `$destroy` listener on the option.
- the registered `$destroy` listener calls `$render` on `ngModel`.
- $render calls `selectCtrl.writeValue()`, which accesses the `options`
object in the `ngOptions` directive.
- Since `ngOptions` has not yet completed linking the `options` has not
yet been defined and we get an error.

This fix moves the registration code for the `option` directive into the
`SelectController`, which can then be easily overridden by the `ngOptions`
directive as a `noop`.

Fixes angular#11685
Closes angular#12972
Closes angular#12968
petebacondarwin pushed a commit to petebacondarwin/angular.js that referenced this issue Oct 5, 2015
When ngOptions is present on a select, the option directive should not be able to
register options on the selectCtrl since this may cause errors during the
ngOptions lifecycle.

This can happen in the following cases:

- there is a blank option below the select element, an ngModel
directive, an ngOptions directive and some other directive on the select
element, which compiles the children of the select
(i.e. the option elements) before ngOptions is has finished linking.

- there is a blank option below the select element, an ngModel
directive, an ngOptions directive and another directive, which uses
templateUrl and replace:true.

What happens is:
- the option directive is compiled and adds an element `$destroy` listener
that will call `ngModel.$render` when the option element is removed.
- when `ngOptions` processes the option, it removes the element, and
triggers the `$destroy` listener on the option.
- the registered `$destroy` listener calls `$render` on `ngModel`.
- $render calls `selectCtrl.writeValue()`, which accesses the `options`
object in the `ngOptions` directive.
- Since `ngOptions` has not yet completed linking the `options` has not
yet been defined and we get an error.

This fix moves the registration code for the `option` directive into the
`SelectController`, which can then be easily overridden by the `ngOptions`
directive as a `noop`.

Fixes angular#11685
Closes angular#12972
Closes angular#12968
Closes angular#13012
petebacondarwin pushed a commit to petebacondarwin/angular.js that referenced this issue Oct 6, 2015
When ngOptions is present on a select, the option directive should not be able to
register options on the selectCtrl since this may cause errors during the
ngOptions lifecycle.

This can happen in the following cases:

- there is a blank option below the select element, an ngModel
directive, an ngOptions directive and some other directive on the select
element, which compiles the children of the select
(i.e. the option elements) before ngOptions is has finished linking.

- there is a blank option below the select element, an ngModel
directive, an ngOptions directive and another directive, which uses
templateUrl and replace:true.

What happens is:
- the option directive is compiled and adds an element `$destroy` listener
that will call `ngModel.$render` when the option element is removed.
- when `ngOptions` processes the option, it removes the element, and
triggers the `$destroy` listener on the option.
- the registered `$destroy` listener calls `$render` on `ngModel`.
- $render calls `selectCtrl.writeValue()`, which accesses the `options`
object in the `ngOptions` directive.
- Since `ngOptions` has not yet completed linking the `options` has not
yet been defined and we get an error.

This fix moves the registration code for the `option` directive into the
`SelectController.registerOption()` method, which is then overridden by
the `ngOptions` directive as a `noop`.

Fixes angular#11685
Closes angular#12972
Closes angular#12968
Closes angular#13012
Narretz added a commit that referenced this issue Oct 6, 2015
When ngOptions is present on a select, the option directive should not be able to
register options on the selectCtrl since this may cause errors during the
ngOptions lifecycle.

This can happen in the following cases:

- there is a blank option below the select element, an ngModel
directive, an ngOptions directive and some other directive on the select
element, which compiles the children of the select
(i.e. the option elements) before ngOptions is has finished linking.

- there is a blank option below the select element, an ngModel
directive, an ngOptions directive and another directive, which uses
templateUrl and replace:true.

What happens is:
- the option directive is compiled and adds an element `$destroy` listener
that will call `ngModel.$render` when the option element is removed.
- when `ngOptions` processes the option, it removes the element, and
triggers the `$destroy` listener on the option.
- the registered `$destroy` listener calls `$render` on `ngModel`.
- $render calls `selectCtrl.writeValue()`, which accesses the `options`
object in the `ngOptions` directive.
- Since `ngOptions` has not yet completed linking the `options` has not
yet been defined and we get an error.

This fix moves the registration code for the `option` directive into the
`SelectController.registerOption()` method, which is then overridden by
the `ngOptions` directive as a `noop`.

Fixes #11685
Closes #12972
Closes #12968
Closes #13012
kkirsche added a commit to kkirsche/kibana that referenced this issue Feb 21, 2016
<a name="1.4.9"></a>
# 1.4.9 implicit-superannuation (2016-01-21)

## Bug Fixes

- **Animation**
  - ensure that animate promises resolve when the document is hidden
  ([9a60408c](angular/angular.js@9a60408))
  - do not trigger animations if the document is hidden
  ([09f6061a](angular/angular.js@09f6061),
   [elastic#12842](angular/angular.js#12842), [elastic#13776](angular/angular.js#13776))
  - only copy over the animation options once
  ([2fc954d3](angular/angular.js@2fc954d),
   [elastic#13722](angular/angular.js#13722), [elastic#13578](angular/angular.js#13578))
  - allow event listeners on document in IE
  ([5ba4419e](angular/angular.js@5ba4419),
   [elastic#13548](angular/angular.js#13548), [elastic#13696](angular/angular.js#13696))
  - allow removing classes that are added by a running animation
  ([6c4581fc](angular/angular.js@6c4581f),
   [elastic#13339](angular/angular.js#13339), [elastic#13380](angular/angular.js#13380), [elastic#13414](angular/angular.js#13414), [elastic#13472](angular/angular.js#13472), [elastic#13678](angular/angular.js#13678))
  - do not use `event.timeStamp` anymore for time tracking
  ([620a20d1](angular/angular.js@620a20d),
   [elastic#13494](angular/angular.js#13494), [elastic#13495](angular/angular.js#13495))
  - ignore children without animation data when closing them
  ([be01cebf](angular/angular.js@be01ceb),
   [elastic#11992](angular/angular.js#11992), [elastic#13424](angular/angular.js#13424))
  - do not alter the provided options data
  ([7a81e6fe](angular/angular.js@7a81e6f),
   [elastic#13040](angular/angular.js#13040), [elastic#13175](angular/angular.js#13175))
  - correctly handle `$animate.pin()` host elements
  ([a985adfd](angular/angular.js@a985adf),
   [elastic#13783](angular/angular.js#13783))
  - allow animations when pinned element is parent element
  ([4cb8ac61](angular/angular.js@4cb8ac6),
   [elastic#13466](angular/angular.js#13466))
  - allow enabled children to animate on disabled parents
  ([6d85f24e](angular/angular.js@6d85f24),
   [elastic#13179](angular/angular.js#13179), [elastic#13695](angular/angular.js#13695))
  - correctly access `minErr`
  ([0c1b54f0](angular/angular.js@0c1b54f))
  - ensure animate runner is the same with and without animations
  ([937942f5](angular/angular.js@937942f),
   [elastic#13205](angular/angular.js#13205), [elastic#13347](angular/angular.js#13347))
  - remove animation end event listeners on close
  ([d9157849](angular/angular.js@d915784),
   [elastic#13672](angular/angular.js#13672))
  - consider options.delay value for closing timeout
  ([592bf516](angular/angular.js@592bf51),
   [elastic#13355](angular/angular.js#13355), [elastic#13363](angular/angular.js#13363))
- **$controller:** allow identifiers containing `$`
  ([2563ff7b](angular/angular.js@2563ff7),
   [elastic#13736](angular/angular.js#13736))
- **$http:** throw if url passed is not a string
  ([c5bf9dae](angular/angular.js@c5bf9da),
   [elastic#12925](angular/angular.js#12925), [elastic#13444](angular/angular.js#13444))
- **$parse:** handle interceptors with `undefined` expressions
  ([7bb2414b](angular/angular.js@7bb2414))
- **$resource:** don't allow using promises as `timeout` and log a warning
  ([47486524](angular/angular.js@4748652))
- **formatNumber:** cope with large and small number corner cases
  ([9c49eb13](angular/angular.js@9c49eb1),
   [elastic#13394](angular/angular.js#13394), [elastic#8674](angular/angular.js#8674), [elastic#12709](angular/angular.js#12709), [elastic#8705](angular/angular.js#8705), [elastic#12707](angular/angular.js#12707), [elastic#10246](angular/angular.js#10246), [elastic#10252](angular/angular.js#10252))
- **input:**
  - fix URL validation being too strict
  ([6610ae81](angular/angular.js@6610ae8),
   [elastic#13528](angular/angular.js#13528), [elastic#13544](angular/angular.js#13544))
  - add missing chars to URL validation regex
  ([2995b54a](angular/angular.js@2995b54),
   [elastic#13379](angular/angular.js#13379), [elastic#13460](angular/angular.js#13460))
- **isArrayLike:** recognize empty instances of an Array subclass
  ([323f9ab7](angular/angular.js@323f9ab),
   [elastic#13560](angular/angular.js#13560), [elastic#13708](angular/angular.js#13708))
- **ngInclude:** do not compile template if original scope is destroyed
  ([9590bcf0](angular/angular.js@9590bcf))
- **ngOptions:**
  - don't skip `optgroup` elements with `value === ''`
  ([85e392f3](angular/angular.js@85e392f),
   [elastic#13487](angular/angular.js#13487), [elastic#13489](angular/angular.js#13489))
  - don't `$dirty` multiple select after compilation
  ([f163c905](angular/angular.js@f163c90),
   [elastic#13211](angular/angular.js#13211), [elastic#13326](angular/angular.js#13326))
- **select:** re-define `ngModelCtrl.$render` in the `select` directive's postLink function
  ([529b2507](angular/angular.js@529b250),
   [elastic#13583](angular/angular.js#13583), [elastic#13583](angular/angular.js#13583), [elastic#13663](angular/angular.js#13663))

## Minor Features

- **ngLocale:** add support for standalone months
  ([54c4041e](angular/angular.js@54c4041),
   [elastic#3744](angular/angular.js#3744), [elastic#10247](angular/angular.js#10247), [elastic#12642](angular/angular.js#12642), [elastic#12844](angular/angular.js#12844))
- **ngMock:** add support for `$animate.closeAndFlush()`
  ([512c0811](angular/angular.js@512c081))

## Performance Improvements

- **ngAnimate:** speed up `areAnimationsAllowed` check
  ([2d3303dd](angular/angular.js@2d3303d))

## Breaking Changes

While we do not deem the following to be a real breaking change we are highlighting it here in the
changelog to ensure that it does not surprise anyone.

- **$resource:** due to [47486524](angular/angular.js@4748652),

**Possible breaking change** for users who updated their code to provide a `timeout`
promise for a `$resource` request in version v1.4.8.

Up to v1.4.7 (included), using a promise as a timeout in `$resource`, would silently
fail (i.e. have no effect).

In v1.4.8, using a promise as timeout would have the (buggy) behaviour described
in angular/angular.js#12657 (comment).
(I.e. it will work as expected for the first time you resolve the promise and will
cancel all subsequent requests after that - one has to re-create the resource
class. This was not documented.)

With this change, using a promise as timeout in v1.4.9 onwards is not allowed.
It will log a warning and ignore the timeout value.

If you need support for cancellable `$resource` actions, you should upgrade to
version 1.5 or higher.

<a name="1.4.8"></a>
# 1.4.8 ice-manipulation (2015-11-19)

## Bug Fixes

- **$animate:** ensure leave animation calls `close` callback
  ([6bd6dbff](angular/angular.js@6bd6dbf),
   [elastic#12278](angular/angular.js#12278), [elastic#12096](angular/angular.js#12096), [elastic#13054](angular/angular.js#13054))
- **$cacheFactory:** check key exists before decreasing cache size count
  ([2a5a52a7](angular/angular.js@2a5a52a),
   [elastic#12321](angular/angular.js#12321), [elastic#12329](angular/angular.js#12329))
- **$compile:**
  - bind all directive controllers correctly when using `bindToController`
  ([5d8861fb](angular/angular.js@5d8861f),
   [elastic#11343](angular/angular.js#11343), [elastic#11345](angular/angular.js#11345))
  - evaluate against the correct scope with bindToController on new scope
  ([b9f7c453](angular/angular.js@b9f7c45),
   [elastic#13021](angular/angular.js#13021), [elastic#13025](angular/angular.js#13025))
  - fix scoping of transclusion directives inside replace directive
  ([74da0340](angular/angular.js@74da034),
   [elastic#12975](angular/angular.js#12975), [elastic#12936](angular/angular.js#12936), [elastic#13244](angular/angular.js#13244))
- **$http:** apply `transformResponse` even when `data` is empty
  ([c6909464](angular/angular.js@c690946),
   [elastic#12976](angular/angular.js#12976), [elastic#12979](angular/angular.js#12979))
- **$location:** ensure `$locationChangeSuccess` fires even if URL ends with `#`
  ([6f8ddb6d](angular/angular.js@6f8ddb6),
   [elastic#12175](angular/angular.js#12175), [elastic#13251](angular/angular.js#13251))
- **$parse:** evaluate once simple expressions only once
  ([e4036824](angular/angular.js@e403682),
   [elastic#12983](angular/angular.js#12983), [elastic#13002](angular/angular.js#13002))
- **$resource:** allow XHR request to be cancelled via a timeout promise
  ([7170f9d9](angular/angular.js@7170f9d),
   [elastic#12657](angular/angular.js#12657), [elastic#12675](angular/angular.js#12675), [elastic#10890](angular/angular.js#10890), [elastic#9332](angular/angular.js#9332))
- **$rootScope:** prevent IE9 memory leak when destroying scopes
  ([87b0055c](angular/angular.js@87b0055),
   [elastic#10706](angular/angular.js#10706), [elastic#11786](angular/angular.js#11786))
- **Angular.js:** fix `isArrayLike` for unusual cases
  ([70edec94](angular/angular.js@70edec9),
   [elastic#10186](angular/angular.js#10186), [elastic#8000](angular/angular.js#8000), [elastic#4855](angular/angular.js#4855), [elastic#4751](angular/angular.js#4751), [elastic#10272](angular/angular.js#10272))
- **isArrayLike:** handle jQuery objects of length 0
  ([d3da55c4](angular/angular.js@d3da55c))
- **jqLite:**
  - deregister special `mouseenter` / `mouseleave` events correctly
  ([22f66025](angular/angular.js@22f6602),
   [elastic#12795](angular/angular.js#12795), [elastic#12799](angular/angular.js#12799))
  - ensure mouseenter works with svg elements on IE
  ([c1f34e8e](angular/angular.js@c1f34e8),
   [elastic#10259](angular/angular.js#10259), [elastic#10276](angular/angular.js#10276))
- **limitTo:** start at 0 if `begin` is negative and exceeds input length
  ([4fc40bc9](angular/angular.js@4fc40bc),
   [elastic#12775](angular/angular.js#12775), [elastic#12781](angular/angular.js#12781))
- **merge:**
  - ensure that jqlite->jqlite and DOM->DOM
  ([2f8db1bf](angular/angular.js@2f8db1b))
  - clone elements instead of treating them like simple objects
  ([838cf4be](angular/angular.js@838cf4b),
   [elastic#12286](angular/angular.js#12286))
- **ngAria:** don't add tabindex to radio and checkbox inputs
  ([59f1f4e1](angular/angular.js@59f1f4e),
   [elastic#12492](angular/angular.js#12492), [elastic#13095](angular/angular.js#13095))
- **ngInput:** change URL_REGEXP to better match RFC3987
  ([cb51116d](angular/angular.js@cb51116),
   [elastic#11341](angular/angular.js#11341), [elastic#11381](angular/angular.js#11381))
- **ngMock:** reset cache before every test
  ([91b7cd9b](angular/angular.js@91b7cd9),
   [elastic#13013](angular/angular.js#13013))
- **ngOptions:**
  - skip comments and empty options when looking for options
  ([0f58334b](angular/angular.js@0f58334),
   [elastic#12190](angular/angular.js#12190), [elastic#13029](angular/angular.js#13029), [elastic#13033](angular/angular.js#13033))
  - override select option registration to allow compilation of empty option
  ([7b2ecf42](angular/angular.js@7b2ecf4),
   [elastic#11685](angular/angular.js#11685), [elastic#12972](angular/angular.js#12972), [elastic#12968](angular/angular.js#12968), [elastic#13012](angular/angular.js#13012))

## Performance Improvements

- **$compile:** use static jquery data method to avoid creating new instances
  ([55ad192e](angular/angular.js@55ad192))
- **copy:**
  - avoid regex in `isTypedArray`
  ([19fab4a1](angular/angular.js@19fab4a))
  - only validate/clear if the user specifies a destination
  ([d1293540](angular/angular.js@d129354),
   [elastic#12068](angular/angular.js#12068))
- **merge:** remove unnecessary wrapping of jqLite element
  ([ce6a96b0](angular/angular.js@ce6a96b),
   [elastic#13236](angular/angular.js#13236))

## Breaking Changes

Signed-off-by: Kevin Kirsche <[email protected]>
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.

8 participants