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

Feature request: backport node patch for Array#values #1897

Closed
ljharb opened this issue Jun 4, 2015 · 24 comments
Closed

Feature request: backport node patch for Array#values #1897

ljharb opened this issue Jun 4, 2015 · 24 comments
Labels
feature request Issues that request new features to be added to Node.js. v8 engine Issues and PRs related to the V8 dependency.

Comments

@ljharb
Copy link
Member

ljharb commented Jun 4, 2015

The Array#values function was temporarily removed from v8 because it broke web compatibility.

node/io.js, however, do not have that concern.

Per nodejs/node-v0.x-archive#25324 (comment), if io.js merges in these two commits, io.js (and future versions of node) will retain Array.prototype.values rather than being hindered by web compat concerns.

Can this be brought in to the next non-patch release of io.js?

@mscdex mscdex added the v8 engine Issues and PRs related to the V8 dependency. label Jun 4, 2015
@bnoordhuis
Copy link
Member

We have a policy of only carrying patches that have been accepted upstream. Here, it's been reverted upstream and it's unclear when it's going to get added back.

Minor aside, array.values() is identical to array[Symbol.iterator]() and that still works in V8. They're literally the same function.

@ljharb
Copy link
Member Author

ljharb commented Jun 4, 2015

:-/ how will node's policy of having this patch, and io.js's policy of not allowing it, work together in the upcoming merge?

Yes, you're correct - by spec, they're === to each other. However, that doesn't change the fact that having .values makes arrays in line with Map and Set (also the weak varieties), and also is a clearer semantic for some than referencing Symbol.iterator.

@Fishrock123
Copy link
Contributor

Uh oh, not good.

The node patch should have floated a version that warned about being deprecated.

@Fishrock123
Copy link
Contributor

Ok I just checked, and it's in the spec for es6, so it will eventually land. When? That's up to v8. :S

@ljharb
Copy link
Member Author

ljharb commented Jun 4, 2015

Yes, exactly - this function is required by the spec, and v8 is only holding it back for web compat. I'd say that v8 should keep it in and Chrome should remove it, but they didn't do it that way for whatever reason.

@Fishrock123
Copy link
Contributor

I'd say that v8 should keep it in and Chrome should remove it, but they didn't do it that way for whatever reason.

Wasn't the 0.12 v8 version using some sort of wrong (non-v8-release-line) v8 version?

If so, that's kinda the problem. :(

@trevnorris
Copy link
Contributor

@Fishrock123 It was, but has been fixed.

@Fishrock123
Copy link
Contributor

It was, but has been fixed.

Right but there were still releases in the wild with this Array feature in from them being dev versions. :/ (In the past now, but I suspect this will just be a general ouch until v8 adds it again.)

@Slayer95
Copy link

Slayer95 commented Jun 5, 2015

ES6 hasn't been approved yet afaik. Given that Array#values breaks web-compatibility, it wouldn't be harebrained to think that it may get rejected in this month's meeting.

@ljharb
Copy link
Member Author

ljharb commented Jun 5, 2015

@Slayer95 Yes, it most certainly has been, and is finalized - as of last week's (May) TC39 meeting. Array#values is definitely in ES6.

@rlidwka
Copy link
Contributor

rlidwka commented Jun 7, 2015

:-/ how will node's policy of having this patch, and io.js's policy of not allowing it, work together in the upcoming merge?

I would expect to see that patch in node 0.12 branch for b/w compatibility reasons, but io.js (and future converged repo) should wait until v8 re-adds it.

@domenic
Copy link
Contributor

domenic commented Jun 9, 2015

-1, should not add features that are not agreed to be added by V8 (and SpiderMonkey). It may be in the ES2015 spec but that doesn't mean it'll be part of the actual JavaScript language accepted and implemented by engines.

@Fishrock123 Fishrock123 added the feature request Issues that request new features to be added to Node.js. label Jun 10, 2015
@Fishrock123
Copy link
Contributor

I think I'm going to close this for now. I would consider it a bug that it even existed unflagged in node (without being stable yet).

@ChALkeR
Copy link
Member

ChALkeR commented Jun 10, 2015

This should be mentioned in the changelog of the converged release, I think.
Among with other breaking changes from 0.12.

@Fishrock123
Copy link
Contributor

Will do.

@gsathya
Copy link
Member

gsathya commented Apr 11, 2018

I re enabled Array.prototype.values in Chrome 66 with an embedder flag. We'll know if it's web compatible or not once Chrome 66 becomes stable.

cc @MylesBorins

@MylesBorins MylesBorins reopened this Apr 11, 2018
@MylesBorins
Copy link
Contributor

Reopening this issue to track. We should likely enable it in 10.x assuming it doesn't get reverted again. @gsathya does it make sense for us to enable it a couple weeks later or perhaps inherit it in the 6.7 upgrade?

@gsathya
Copy link
Member

gsathya commented Apr 11, 2018

I think waiting for 6.7 is a good idea. I'll enable it by default in V8 6.8 and backport it to V8 6.7 (since 6.7 is branching right now)

@targos
Copy link
Member

targos commented Apr 27, 2018

Friendly ping. It looks it's still behind a flag in 6.8?

@gsathya
Copy link
Member

gsathya commented Apr 27, 2018

Thanks @targos. We're still ramping up Chrome 66 release rollout, so I don't have enough data yet to know if it's web compatible.

@Trott
Copy link
Member

Trott commented Nov 11, 2018

Seems like Array.prototype.values() is available in Node.js 10.x and 11.x. It's not there in 8.x and 6.x. Is this close-able on the grounds that it's unlikely to be backported to 8.x and 6.x? Or is that precisely the ask here and it's something that's still wanted?

@ljharb
Copy link
Member Author

ljharb commented Nov 11, 2018

It should still be backported if possible, imo.

@bnoordhuis
Copy link
Member

v6.x is EOL and v8.x is in maintenance mode now. I'd say it's exceedingly unlikely that a back-port will be accepted at this stage because it doesn't meet the criteria:

Once a release moves into Maintenance mode, only critical bugs, critical security fixes [..] will be permitted.

Ergo, I'll go ahead and close this out.

@ljharb
Copy link
Member Author

ljharb commented Mar 29, 2019

Certainly dragging our heels on it for 4 or 1 year makes it obsolete, but that doesn’t mean it’s highly subpar that it wasn’t backported. I hope we won’t be so glacial with such things in the future.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request Issues that request new features to be added to Node.js. v8 engine Issues and PRs related to the V8 dependency.
Projects
None yet
Development

No branches or pull requests