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

RESOLVED FIXED in Firefox 43

Status

()

RESOLVED FIXED
5 years ago
4 years ago

People

(Reporter: till, Assigned: till)

Tracking

(Blocks: 2 bugs, {dev-doc-complete})

Trunk
mozilla43
dev-doc-complete
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(firefox43 fixed)

Details

(Whiteboard: [DocArea=JS])

Attachments

(1 attachment)

(Assignee)

Description

5 years ago
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]
(Assignee)

Updated

5 years ago
Blocks: 1072889
(Assignee)

Updated

4 years ago
Summary: Enable Array.prototype.contains in all builds → Enable Array.prototype.includes in all builds
(Assignee)

Comment 1

4 years ago
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.

Updated

4 years ago
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Component: JavaScript Engine → JavaScript: Standard Library
Keywords: dev-doc-needed → dev-doc-complete
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 → ---

Comment 3

4 years ago
(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)
(Assignee)

Comment 4

4 years ago
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)
(Assignee)

Comment 5

4 years ago
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)

Comment 6

4 years ago
(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)

Updated

4 years ago
Depends on: 1111869
Summary: Enable Array.prototype.includes in all builds → Enable {Array, %TypedArray%}.prototype.includes in all builds
Keywords: dev-doc-complete → dev-doc-needed

Updated

4 years ago
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.

Comment 8

4 years ago
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

Comment 9

4 years ago
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.
(Assignee)

Comment 10

4 years ago
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)

Updated

4 years ago
Assignee: nobody → till
Status: NEW → ASSIGNED
(Assignee)

Comment 12

4 years ago
(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+
(Assignee)

Comment 13

4 years ago
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
(Assignee)

Comment 14

4 years ago
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
Last Resolved: 4 years ago4 years ago
status-firefox43: --- → fixed
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla43
(Assignee)

Updated

4 years ago
Keywords: dev-doc-needed → dev-doc-complete
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
Keywords: dev-doc-complete → dev-doc-needed
and the 43 site compatibility doc as well. (I'll work on it)
This has been added to the 43 developer release doc.
Keywords: dev-doc-needed → dev-doc-complete
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.