1216751 - provide forEach() method on iterable<> webidl interfaces

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Ben Kelly [:bkelly, not reviewing]]

This part of the webidl spec suggests iterable<> interfaces should have a forEach() method:

  http://heycam.github.io/webidl/#es-forEach

It seems there might be some issues with the spec, though.

I think we want to get this resolved, though, as most of the real world code I've seen using iterable on fetch Headers tries to use forEach().

Comment 1

[Boris Zbarsky [:bzbarsky]]

Note to self:  this spec needs a bit of work, since k and kValue are IDL values, not ES values, so they need conversion to ES values before the callback function can be invoked....  In implementation terms, we can either do that in bindings by hand or make the generated forEach method take as its first argument a callback type we synthesize which takes the right IDL arguments and let the binding infrastructure do most of the work.

Comment 2

[Boris Zbarsky [:bzbarsky]]

Oh, and the current spec only makes sense for single-type iterables.  And it seems to get the arguments backwards from every single other forEach in the world (which all have the value first, then the key).

I guess I'll try to write up a sane spec here....

Comment 3

[Boris Zbarsky [:bzbarsky]]

Oh, and the third arg to forEach should be the this value; the callback this value should be undefined...  At least if we want to match every iterable in ES6 that has forEach, as well as Chrome's Headers impl.

Comment 4

[Boris Zbarsky [:bzbarsky]]

Though actually, do we want more arraylike semantics for forEach or more setlike?  In particular, does a removal of a key at or before the point where forEach is called cause some keys to be skipped (like array) or not (like map/set)?

Chrome's Headers impl seems more arraylike in this case:

  var h = new Headers;
  h.append("a", "b")
  h.append("c", "d")
  h.append("e", "f")
  h.forEach(function(...args) {
    console.log(args);
    h.delete("a");
  });

logs:

  ["b", "a", Headers]
  ["f", "e", Headers]

I'm going to start a thread on public-script-coord, because this seems like the sort of thing that actually needs figuring out. :(

Comment 5

[Boris Zbarsky [:bzbarsky]]

Thread at https://lists.w3.org/Archives/Public/public-script-coord/2015OctDec/0039.html

Comment 6

[Jean-Yves Perrier [:teoli]]

Added dev-doc-needed.

When this lands, we have to document forEach() for all occurences of iterator<>, currently FormData, Headers and URLSearchParam.

We will need to update: https://developer.mozilla.org/en-US/docs/MDN/Contribute/Howto/Write_an_API_reference/Information_contained_in_a_WebIDL_file#Pair_iterator 
(removing the link to this bug and linking to FormData.forEach().

Comment 7

[Chris Peterson [:cpeterson]]

Cameron merged his spec changes for iterable<> yesterday:

https://github.com/heycam/webidl/commit/3c61abe1dda3c157ddf278b312540702277ecc00

/me assigning to Boris because he said he plans to fix this bug this week.

Comment 8

[Boris Zbarsky [:bzbarsky]]

Attachment: part 1. Restrict value iterators to interfaces that have indexed properties and pair iterators to interfaces that do not have indexed properties

Comment 9

[Boris Zbarsky [:bzbarsky]]

Attachment: part 2. For value iterators, "entries", "keys", and "values" must just come from Array.prototype

Comment 10

[Boris Zbarsky [:bzbarsky]]

Attachment: part 3. For pair iterators, @@iterator should be an alias for "values". Similarly for maplikes and "entries" and setlikes and "values"

Comment 11

[Boris Zbarsky [:bzbarsky]]

Attachment: part 4. Implement forEach for iterable interfaces

Comment 12

[Boris Zbarsky [:bzbarsky]]

Attachment: part 5. Remove the now-unnecessary value iterator infrastructure, since it's entirely handled via the %ArrayPrototype% methods now

Comment 13

[Boris Zbarsky [:bzbarsky]]

Argh, this fails tests, because the spec is still wrong.  In particular, https://github.com/heycam/webidl/issues/67 still needs to get fixed...  Updated patches coming up.

Comment 14

[Boris Zbarsky [:bzbarsky]]

Attachment: part 3. For pair iterators, @@iterator should be an alias for "entries". Similarly for maplikes and "entries" and setlikes and "values"

Comment 15

[Boris Zbarsky [:bzbarsky]]

I think https://github.com/heycam/webidl/pull/90 should spec what I've implemented here.

Comment 16

[Boris Zbarsky [:bzbarsky]]

Attachment: part 1. Restrict value iterators to interfaces that have indexed properties and pair iterators to interfaces that do not have indexed properties

Comment 17

[Boris Zbarsky [:bzbarsky]]

Attachment: part 2. For value iterators, "entries", "keys", and "values" must just come from Array.prototype

Comment 18

[Boris Zbarsky [:bzbarsky]]

Attachment: part 5. Remove the now-unnecessary value iterator infrastructure, since it's entirely handled via the %ArrayPrototype% methods now

Comment 19

[Kyle Machulis [:qdot] [:kmachulis] (INACTIVE)]

Everything looks good (and simpler/smaller than before!). Sorry for the review delay.

Comment 20

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/5fc801a87404
https://hg.mozilla.org/integration/mozilla-inbound/rev/efad6236ce3f
https://hg.mozilla.org/integration/mozilla-inbound/rev/62d0120fca7d
https://hg.mozilla.org/integration/mozilla-inbound/rev/3ad43093c8f4
https://hg.mozilla.org/integration/mozilla-inbound/rev/410ef34da2e7

Comment 21

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/5fc801a87404
https://hg.mozilla.org/mozilla-central/rev/efad6236ce3f
https://hg.mozilla.org/mozilla-central/rev/62d0120fca7d
https://hg.mozilla.org/mozilla-central/rev/3ad43093c8f4
https://hg.mozilla.org/mozilla-central/rev/410ef34da2e7

Comment 22

[Boris Zbarsky [:bzbarsky]]

Chris, are we going to need this on branches, or is trunk ok?

Comment 23

[Chris Pearce [:cpearce (Not reading bugmail)]]

bz: At this stage, trunk is OK. We may want it uplifted later, I'll follow up if we need it uplifted.