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

Using orderBy on an object should cause an error or warning #11255

Closed
ExplodingCabbage opened this issue Mar 5, 2015 · 2 comments
Closed

Using orderBy on an object should cause an error or warning #11255

ExplodingCabbage opened this issue Mar 5, 2015 · 2 comments

Comments

@ExplodingCabbage
Copy link

Over in #1286 it was decided that orderBy should not be modified to work when doing an ng-repeat over an object.

However, trying to use orderBy on an object doesn't give any clue as to what you're doing wrong; the ordering simply silently fails to take effect: http://jsfiddle.net/uo7kym1n/1/

I just got bitten by this behaviour and the idea that using orderBy on an object was wrong was certainly not the first idea that came to my mind. I suspect the 50 people who +1ed #1286 are likewise people who lost time trying to make sense of why their orderBy wasn't working without Angular offering them any clues.

Would it not be reasonable to have orderBy throw an error or warning when used on an object, instructing that it should not be so used?

@petebacondarwin
Copy link
Contributor

That sounds reasonable... Let's look to add that in 1.5.

@nhodges
Copy link
Contributor

nhodges commented Apr 25, 2015

@ExplodingCabbage @petebacondarwin Would love to get your input on the above PR.

@petebacondarwin petebacondarwin modified the milestones: 1.5.x - migration-facilitation, Backlog Jun 30, 2015
nhodges added a commit to nhodges/angular.js that referenced this issue Oct 28, 2015
gkalpak pushed a commit to gkalpak/angular.js that referenced this issue Nov 23, 2015
BREAKING CHANGE:
Previously, an non array-like input would pass through the orderBy filter
unchanged.
Now, an error is thrown. This can be worked around by converting an object
to an array, either manually or using a filter such as
https://github.com/petebacondarwin/angular-toArrayFilter.
(`null` and `undefined` still pass through without an error, in order to
support asynchronous loading of resources.)

Closes angular#11255
Closes angular#11719
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.

3 participants