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's enumerable contains should align with ES7 Array#contains. #5670

Closed
jdalton opened this issue Oct 1, 2014 · 9 comments
Closed

Ember's enumerable contains should align with ES7 Array#contains. #5670

jdalton opened this issue Oct 1, 2014 · 9 comments

Comments

@jdalton
Copy link

jdalton commented Oct 1, 2014

At the moment Ember's contains does not match NaN values or support a fromIndex like the ES7 Array#contains does. This can cause an inconsistency when Ember adds Array.prototype.contains.

@fivetanley
Copy link
Member

👍 thanks! we'll get it fixed asap

@jdalton
Copy link
Author

jdalton commented Oct 1, 2014

Rock! I made a jsbin for an unrelated Underscore issue that shows this. Just open the web console and try [NaN].contains(NaN) in a non-Firefox-Nightly browser to see it report false. Then in Firefox Nightly to see it report true (as it's using native Array#contains in FF nightly).

@stefanpenner
Copy link
Member

@jdalton thanks for the headsup, we appreciate it!

@stefanpenner
Copy link
Member

i have a fix, will push once i get to work.

stefanpenner added a commit to stefanpenner/ember.js that referenced this issue Oct 2, 2014
stefanpenner added a commit to stefanpenner/ember.js that referenced this issue Oct 4, 2014
@wagenet
Copy link
Member

wagenet commented Nov 1, 2014

@stefanpenner can this be closed?

@stefanpenner
Copy link
Member

I forgot about #9262, will complete

@xtian
Copy link
Contributor

xtian commented Nov 22, 2014

This should be changed to Enumerable#includes and aliased to contains for back-compat as the spec has changed: https://github.com/tc39/Array.prototype.includes#status

@jdalton
Copy link
Author

jdalton commented Nov 22, 2014

Ya, though might hold off a bit. As far as I know the TC39 chose includes as an alternative without doing any crawling to estimate potential conflicts so it will be an implement-and-see situation similar to contains.

@wycats
Copy link
Member

wycats commented Jan 13, 2015

@jdalton 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants