Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ember.computed.sort fails on 2.x with null list (worked in 1.13) #12207

Closed
workmanw opened this issue Aug 26, 2015 · 6 comments
Closed

Ember.computed.sort fails on 2.x with null list (worked in 1.13) #12207

workmanw opened this issue Aug 26, 2015 · 6 comments
Labels

Comments

@workmanw
Copy link

When the target of Ember.computed.sort is null it causes a crash on the 2.x branch. It worked on the Ember 1.13.x branch.

Ember 1.13.9: http://emberjs.jsbin.com/mejidiniju/1/edit
Ember 2.1-beta.2: http://emberjs.jsbin.com/pujaxeceqe/1/edit

@stefanpenner
Copy link
Member

Should be fixed on master

@fivetanley
Copy link
Member

@fivetanley fivetanley added the Bug label Aug 26, 2015
@workmanw
Copy link
Author

@stefanpenner @fivetanley Seems like the easiest solution is to check items for null and bail early here: https://github.com/emberjs/ember.js/blob/master/packages/ember-runtime/lib/computed/reduce_computed_macros.js#L573

I can submit a pull if you think that's the way to go. I'm not really up on Ember's style guide anymore :(

@rwjblue
Copy link
Member

rwjblue commented Aug 26, 2015

We did a similar fix to Ember.computed.sum in #12170. Seems like we need to add a similar guard to Ember.computed.sort.

@workmanw - Can you take a look at the PR above, and submit a PR?

@stefanpenner
Copy link
Member

Ah sort not map

@workmanw
Copy link
Author

@rwjblue @stefanpenner Yea, I'll make it so.

mixonic added a commit that referenced this issue Aug 26, 2015
[BUGFIX release] Ember.computed.sort was crashing when it hit a null value. Fixes #12207.
rwjblue pushed a commit that referenced this issue Sep 1, 2015
rwjblue pushed a commit that referenced this issue Sep 1, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants