8000 Make _.include/_.contains work with strings. by ianb · Pull Request #667 · jashkenas/underscore · GitHub
[go: up one dir, main page]
Skip to content

Make _.include/_.contains work with strings. #667

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 2 commits into from
Closed

Make _.include/_.contains work with strings. #667

wants to merge 2 commits into from

Conversation

@ianb
Copy link
@ianb ianb commented Jul 11, 2012

This makes _.include() / _.contains() work with strings in addition to collections.

Note String.contains is a proposed method for Harmony: http://wiki.ecmascript.org/doku.php?id=harmony:string_extras – also it's really just another indexOf check, so it seems to make sense.

String.indexOf is present in Javascript 1.0, so a method identity check seemed unnecessary.

I wasn't sure where a doc change would go.

@ianb
Copy link
Author
ianb commented Jul 11, 2012

Also, this allows Javascript to naturally coerce the search item to a string. Arguably that's different than how _.include works with Arrays (since it uses === which does not allow this kind of coercion). This is the simplest implementation, but I'd also be comfortable with a type check on target (returning false if the target is not a string).

@ianb
Copy link
Author
ianb commented Jul 11, 2012

I also can't figure out why this works with boxed strings, though it seems to (like new String('test')), since typeof new String('test') == 'object'

@ghost
Copy link
ghost commented Jul 11, 2012

I also can't figure out why this works with boxed strings, though it seems to (like new String('test')), since < 8000 code class="notranslate">typeof new String('test') == 'object'.

Use _.isString instead of typeof.

@jdalton
Copy link
Contributor
jdalton commented Jul 11, 2012

Strings are array-like so that's why this works in modern browsers on the edge version of Underscore.

_.contains('test', 'e'); // => true

@ianb
Copy link
Author
ianb commented Jul 11, 2012

_.contains (in current underscore) didn't seem to work because of the test if (nativeIndexOf && obj.indexOf === nativeIndexOf) return obj.indexOf(target) != -1; where nativeIndexOf = ArrayProto.indexOf

Note:

_contains('test', 'es'); // => false

@ianb
Fix test for strings (to also catch boxed strings). And apparently th…
5dd91ce 
…e test related to boxed strings was backwards.
@jdalton
Copy link
Contributor
jdalton commented Jul 11, 2012

Note:
_.contains('test', 'es'); // => false

Yap, that's because being array-like means having a length and index properties like "hi"[1]; // i.
That said I dig me some ES6 sugar :P

@jashkenas
Copy link
Owner
jashkenas commented Aug 31, 2012

Closing as discussed in #668...

@jashkenas jashkenas closed this Aug 31, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@ianb @jdalton @jashkenas
0