Closed Bug 1070767 Opened 10 years ago Closed 9 years ago

Enable {Array, %TypedArray%}.prototype.includes in all builds

Categories

(Core :: JavaScript: Standard Library, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla43
Tracking Status
firefox43 --- fixed

People

(Reporter: till, Assigned: till)

References

Details

(Keywords: dev-doc-complete, Whiteboard: [DocArea=JS])

Attachments

(1 file)

When the spec is declared stable enough, we should enable contains sooner rather than later. That entails removing the `#ifdef NIGHTLY_BUILD` block in jsarray.cpp and making the tests in "tests/ecma_7/Array.contains.js" and "js/xpconnect/tests/chrome/test_xrayToJS.xul" unconditional.
Keywords: dev-doc-needed
Whiteboard: [DocArea=JS]
Depends on: 1075059
Blocks: 1072889
Summary: Enable Array.prototype.contains in all builds → Enable Array.prototype.includes in all builds
At the current TC39 meeting, it has been decided to rename this to `includes` to avoid web-compat issues with the old name. See bug 1075059.
Status: NEW → RESOLVED
Closed: 10 years ago
Component: JavaScript Engine → JavaScript: Standard Library
Resolution: --- → FIXED
Target Milestone: --- → mozilla36
According to mozilla.dev.platform, Array.prototype.includes is only enabled in nightly builds so far.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Target Milestone: mozilla36 → ---
(In reply to Dão Gottwald [:dao] from comment #2)
> According to mozilla.dev.platform, Array.prototype.includes is only enabled
> in nightly builds so far.

Sorry, I mistakenly thought that there is no need to make this new named method nightly-only any more.
Till, could you please post a follow-up patch to bug 1069063.
Flags: needinfo?(till)
Yeah, in review now. I very obviously should've caught this in the review :( That being said, you'll want to point out changes like this so as to avoid mistakes by stupid reviewers. ;)

The reason for keeping it Nightly-only is to prevent us from getting into a situation like the one we have with String.prototype.contains now.
Flags: needinfo?(till)
Landed followup: https://hg.mozilla.org/integration/mozilla-inbound/rev/160459853756

If we could now all pretend this didn't happen ... (Which entails changing the documentation to say this is Nightly-only. Ziyunfei, can you do that?)
Status: REOPENED → NEW
Flags: needinfo?(446240525)
(In reply to Till Schneidereit [:till] from comment #5)
> Landed followup:
> https://hg.mozilla.org/integration/mozilla-inbound/rev/160459853756
> 
> If we could now all pretend this didn't happen ... (Which entails changing
> the documentation to say this is Nightly-only. Ziyunfei, can you do that?)

Sure.
Flags: needinfo?(446240525)
Depends on: 1111869
Summary: Enable Array.prototype.includes in all builds → Enable {Array, %TypedArray%}.prototype.includes in all builds
Depends on: 1136359
This is creating noise every time we merge central to Aurora and someone used .includes() on an Array in central...
Would be possible to put it behind a pref or something like that? also as a suggestion for future introduction of js features that won't ride the Aurora/Beta trains.
Alternately you could ship it :). This feature has reached stage 3 of the TC39 process, and is awaiting two brave implementations to ship it to stable before it can advance to stage 4 and the spec. I just sent the V8 intent to ship: https://groups.google.com/forum/#!topic/v8-users/-a8_8cb6FRI
Update: {Array,%TypedArray%}.prototype.includes just landed in V8, and unless some unpredictable mishap happens, should be in Chrome 47.

WebKit nightly and Safari betas are shipping Array.prototype.includes, although not the TypedArray version it seems.
Lars, you reviewed the patch that made these Nightly-only, so you also get the honor to do the reverse :)

(In reply to Domenic Denicola from comment #8)
> Alternately you could ship it :). This feature has reached stage 3 of the
> TC39 process, and is awaiting two brave implementations to ship it to stable
> before it can advance to stage 4 and the spec. I just sent the V8 intent to
> ship: https://groups.google.com/forum/#!topic/v8-users/-a8_8cb6FRI

I didn't see this comment earlier. Thanks for this, and the update in comment 9. I agree that at this point we should ship this.
Attachment #8651964 - Flags: review?(lhansen)
Assignee: nobody → till
Status: NEW → ASSIGNED
(In reply to Marco Bonardo [::mak] (spotty available until 24 Aug) from comment #7)
> This is creating noise every time we merge central to Aurora and someone
> used .includes() on an Array in central...
> Would be possible to put it behind a pref or something like that? also as a
> suggestion for future introduction of js features that won't ride the
> Aurora/Beta trains.

Putting JS features behind a flag is pretty hard, unfortunately. Also, it's usually not required, really. This case is pretty unique in how it played out, both for the dearly-missed Array#contains and its replacement, Array#includes. I don't think there has been a case that caused even remotely similar amounts of noise in the last few years. In addition, most features we implement at all nowadays are stable spec-wise. The big exceptions I can think of, SIMD and SharedTypedArrays, don't cause any issues by being Nightly-only, as far as I'm aware.
Attachment #8651964 - Flags: review?(lhansen) → review+
url:        https://hg.mozilla.org/integration/mozilla-inbound/rev/fe9f077de757cc7d95e977a2e3704a6e1de1e815
changeset:  fe9f077de757cc7d95e977a2e3704a6e1de1e815
user:       Till Schneidereit <till@tillschneidereit.net>
date:       Tue Aug 25 12:57:21 2015 +0200
description:
Bug 1070767 - Enable {Array, %TypedArray%}.prototype.includes in all builds. r=lth
Thanks for the quick review.

Try run: https://treeherder.mozilla.org/#/jobs?repo=try&tochange=5bfc7fe26e7a&fromchange=6e8354fa7104

The failures were caused by other commits that have since been backed out.
https://hg.mozilla.org/mozilla-central/rev/fe9f077de757
Status: ASSIGNED → RESOLVED
Closed: 10 years ago9 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla43
The developer documentation is not complete yet. It still needs to be mentioned at https://developer.mozilla.org/en-US/Firefox/Releases/43. And that should probably happen after the next release cycle.

Sebastian
and the 43 site compatibility doc as well. (I'll work on it)
This has been added to the 43 developer release doc.
I guess we don't need this in the site compat doc.
You need to log in before you can comment on or make changes to this bug.