Array.prototype[@@iterator] should be the same function object as Array.prototype.values

RESOLVED FIXED in Firefox 48

Status

()

Core
JavaScript Engine
RESOLVED FIXED
4 years ago
9 months ago

People

(Reporter: benvie, Assigned: jorendorff)

Tracking

(Blocks: 1 bug, {dev-doc-complete})

Trunk
mozilla25
dev-doc-complete
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite ?

Firefox Tracking Flags

(firefox48 fixed)

Details

Attachments

(2 attachments, 1 obsolete attachment)

(Reporter)

Description

4 years ago
The ES6 spec states:

> Array.prototype[@@iterator]()
> The initial value of the @@iterator property is the same function object as the initial value of the Array.prototype.values property.

> Map.prototype[@@iterator]()
> The initial value of the @@iterator property is the same function object as the initial value of the Map.prototype.entries property.

> Set.prototype[@@iterator ]()
> The initial value of the @@iterator property is the same function object as the initial value of the Set.prototype.values property.


Ignoring the fact that SpiderMonkey currently isn't using a Symbol for the @@iterator property under the assumption that the string keyed "iterator" is a temporary stand-in for it, these departures from the ES6 spec can be observed:

> Array.prototype.entries !== Array.prototype.iterator;
> Array.prototype.iterator.name !== "entries";

> Map.prototype.entries !== Map.prototype.iterator;
> Map.prototype.iterator.name !== "entries";

(Set.{keys, values, entries} is not implemented yet, bug 869996)
(Reporter)

Comment 1

4 years ago
Apologies, a correction. This should be observed for Array:

> Array.prototype.values !== Array.prototype.iterator;
> Array.prototype.iterator.name !== "values";
Created attachment 754135 [details] [diff] [review]
patch for Array.prototype

This should patch should fix the Array.prototype deviations from the ES6 spec.

The rest of the problem for Map and Set should go away once the patch on the bug 869996 has been landed.
Assignee: general → sankha93
Status: NEW → ASSIGNED
Attachment #754135 - Flags: review?(jorendorff)
Depends on: 869996
(Assignee)

Updated

4 years ago
Attachment #754135 - Flags: review?(jorendorff) → review+
https://hg.mozilla.org/mozilla-central/rev/5d35dc039af7
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla24
Keywords: dev-doc-needed
Depends on: 881782
(Assignee)

Comment 4

4 years ago
Adding Array.prototype.values caused some surprising compatibility issues (bug 881782, bug 883914).  I think we need to do a partial backout before FF24.  I intend to do so tomorrow unless the plot develops further.

I've mentioned this to es-discuss, as well -- the method we added that seems to be causing the problems is standard-track. Presumably that will change.
Depends on: 883914
(Assignee)

Comment 5

4 years ago
Backed out for now. No real discussion on es-discuss yet.
  https://hg.mozilla.org/integration/mozilla-inbound/rev/38f0844975f2
Status: RESOLVED → REOPENED
Resolution: FIXED → ---

Comment 6

4 years ago
(In reply to Jason Orendorff [:jorendorff] from comment #5)
> Backed out for now. No real discussion on es-discuss yet.
>   https://hg.mozilla.org/integration/mozilla-inbound/rev/38f0844975f2

Merge of backout:
https://hg.mozilla.org/mozilla-central/rev/38f0844975f2
[clearing milestone and in-testsuite, since this has been backed out]
Flags: in-testsuite+ → in-testsuite?
Target Milestone: mozilla24 → ---
(Assignee)

Comment 8

4 years ago
I'm sure Sencha is working on fixing Ext JS but that won't fix existing web sites without further outreach.

The attitude on es-discuss seems to be "wait and see", so this could be in limbo for a long time. Setting need-info from dherman, one of our TC39 members.
Flags: needinfo?(dherman)
(Reporter)

Comment 9

4 years ago
Having Array.prototype.values has limited usefulness as long as you have Array.prototype.@@iterator (or just "iterator" in our case currently). Perhaps we could spin this bug off into two bugs: one for Array.prototype.values which is in limbo, and one for the rest which is not.
(Reporter)

Comment 10

4 years ago
Oh nevermind, Map/Set was done elsewhere. Ignore my last comment.
(Reporter)

Updated

4 years ago
Summary: iterator methods of [Array, Map, Set].prototype should be same function object as entries/values → Array.prototype.iterator should be the same function object as Array.prototype.values
(Assignee)

Comment 11

4 years ago
Trying again, now that FF24 has branched.

https://hg.mozilla.org/integration/mozilla-inbound/rev/b4426d926b31
Please use hg out to check the commit message before you push. This one got a bit munged.
So, is there a plan to address the site regressions from comment 4?  Are those going to become tech evang issues?
(In reply to Daniel Holbert [:dholbert] from comment #13)
> So, is there a plan to address the site regressions from comment 4?  Are
> those going to become tech evang issues?

Sencha is aware of the issue and working on a fix. Once they have that, a roll out to regressing sites is hopefully going to be fairly painless, and hence quick. Hopefully.
https://hg.mozilla.org/mozilla-central/rev/b4426d926b31
Status: REOPENED → RESOLVED
Last Resolved: 4 years ago4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla25

Updated

4 years ago
Depends on: 892225
https://hg.mozilla.org/integration/mozilla-inbound/rev/22c3433bdbc9
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Just for completeness, and because it confused someone on another bug, here's the m-c changeset for Comment 16's backout:
 https://hg.mozilla.org/mozilla-central/rev/22c3433bdbc9
Should this bug be closed (or morphed?) as a result of bug 881782 comment 22?
(Assignee)

Comment 19

4 years ago
No, keep this open.

TC39 is designing a truly gruesome workaround for this. Eventually it'll be done and we'll implement, and then we can re-land the patch here.

Updated

3 years ago
Keywords: site-compat
We can try relanding this once bug 1054759 is fixed.
Depends on: 1054759
Flags: needinfo?(dherman)

Updated

2 years ago
See Also: → bug 1067049
(Assignee)

Updated

a year ago
Depends on: 1258163
Next time let's land this together with bug 1258163.
Assignee: sankha93 → evilpies
Attachment #754135 - Attachment is obsolete: true
Created attachment 8732611 [details] [diff] [review]
Add Array.prototype.values
Attachment #8732611 - Flags: review?(jorendorff)
Summary: Array.prototype.iterator should be the same function object as Array.prototype.values → Array.prototype[@@iterator[ should be the same function object as Array.prototype.values
Summary: Array.prototype[@@iterator[ should be the same function object as Array.prototype.values → Array.prototype[@@iterator] should be the same function object as Array.prototype.values
(Assignee)

Comment 23

a year ago
Comment on attachment 8732611 [details] [diff] [review]
Add Array.prototype.values

Review of attachment 8732611 [details] [diff] [review]:
-----------------------------------------------------------------

r=me. I had this in my patch for a while, but didn't realize we had _SetCanonicalName -- so I didn't expect it to work -- so I removed it. Nice!
Attachment #8732611 - Flags: review?(jorendorff) → review+
(Assignee)

Comment 24

a year ago
I'll land this when I reland @@unscopables, hopefully Monday.
I think I forgot to add values to test_xrayToJS.xul.
(Assignee)

Comment 26

a year ago
Yeah, I expected that. I'll add it before pushing to the try server.
(Assignee)

Comment 27

a year ago
Created attachment 8732889 [details] [diff] [review]
Implement Array.prototype.values
Attachment #8732889 - Flags: review?(bobbyholley)
(Assignee)

Updated

a year ago
Assignee: evilpies → jorendorff
Status: REOPENED → ASSIGNED
(Assignee)

Comment 28

a year ago
https://treeherder.mozilla.org/#/jobs?repo=try&revision=068a8afc645a
Comment on attachment 8732889 [details] [diff] [review]
Implement Array.prototype.values

Review of attachment 8732889 [details] [diff] [review]:
-----------------------------------------------------------------

r=me on the xray bits. Everything else looks fine, but I don't know the spec well enough to confirm that this matches it.
Attachment #8732889 - Flags: review?(bobbyholley) → review+
(Assignee)

Comment 30

a year ago
https://treeherder.mozilla.org/#/jobs?repo=try&revision=d098511959e1
I added the site-compat keyword 2 years ago but is there still any chance of breaking site compatibility here?
(Assignee)

Comment 32

a year ago
Not with @@unscopables in place, no.
Then removing the keyword. Thanks.
Keywords: site-compat
(Assignee)

Comment 34

a year ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/ee890a109435989837df6bf7b860203bbe80ff4b
Bug 875433 - Implement Array.prototype.values. r=jorendorff.

Comment 35

a year ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/ee890a109435
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years agoa year ago
status-firefox48: --- → fixed
Resolution: --- → FIXED
https://developer.mozilla.org/en-US/Firefox/Releases/48#JavaScript
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/values
Keywords: dev-doc-needed → dev-doc-complete

Updated

9 months ago
Depends on: 1299593
You need to log in before you can comment on or make changes to this bug.