Skip to content
This repository has been archived by the owner on Dec 3, 2019. It is now read-only.

Approve ES2017 Array.includes() #3424

Closed
zeptonaut opened this issue Mar 20, 2017 · 4 comments
Closed

Approve ES2017 Array.includes() #3424

zeptonaut opened this issue Mar 20, 2017 · 4 comments
Assignees
Labels

Comments

@zeptonaut
Copy link
Contributor

zeptonaut commented Mar 20, 2017

ES 2017 introduces an Array/String.includes() method that allows you to write:

if ('text1234'.includes('xt12')) {
  ...
}

instead of

if ('text1234'.indexOf('xt12') !== -1) {
  ...
}

This new syntax has already slipped into the codebase without causing any problems. Are there any objections to marking it as approved?

@eakuefner @benshayden

@zeptonaut zeptonaut added the ES6+ label Mar 20, 2017
@zeptonaut zeptonaut self-assigned this Mar 20, 2017
@zeptonaut
Copy link
Contributor Author

Also relevant: the feature is widely implemented (Chrome 47, Firefox 43, Edge 14, Opera 34, Safari 9).

@benshayden
Copy link
Contributor

sgtm, thanks!

(Looks like the link is for indexOf, not includes)

@lpy
Copy link
Contributor

lpy commented Mar 20, 2017

LGTM.
BTW, I guess we really shouldn't post any corp link publicly.

@zeptonaut
Copy link
Contributor Author

@lpy This is definitely the policy. I'm going to start a conversation about this in an internal email thread to figure out how we should handle this wrt. code search, code review, etc.

chromium-infra-bot pushed a commit that referenced this issue Mar 21, 2017
chromium-infra-bot pushed a commit that referenced this issue Mar 22, 2017
A follow-up CL to use this across the codebase is in
progress here: http://crrev.com/2768513002.

BUG=catapult:#3424

Review-Url: https://codereview.chromium.org/2764813005
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

3 participants