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

iterable's should have more than just forEach() #561

Open
marcoscaceres opened this issue May 29, 2018 · 21 comments
Open

iterable's should have more than just forEach() #561

marcoscaceres opened this issue May 29, 2018 · 21 comments

Comments

@marcoscaceres
Copy link
Member

iterable declarations adds.forEach() method to an interface, which is nice... however, only having forEach is too limited - would be great if iterable declarations got all the useful Array methods, like .reduce(), .find(), etc.

@annevk
Copy link
Member

annevk commented May 29, 2018

It's more than forEach():

In the ECMAScript language binding, an interface that is iterable will have entries, forEach, keys, values, and @@iterator properties on its interface prototype object.

But yeah, it's worth investigating if we can add more.

@bzbarsky
Copy link
Collaborator

This has come up a few times.

Realistically, this is why I supported [ArrayClass]: you get all the generic array things for free.... Anyway, we could have iterable declarations pull in Array.prototype.find onto the relevant prototype and that would work fine, I think. And we could do similar for others, and keep whacking the moles as ES gets updated.

@bzbarsky
Copy link
Collaborator

Even nicer would be if ES had facilities for doing this stuff by default on anything that has keys/values/@@iterator, because then it would Just Work. But the current facility for it is Array.prototype.find.call(myObj, stuff) which is not terribly ergonomic.

@TimothyGu
Copy link
Member

Or [...myObj].find(stuff), which doesn't look too bad… (No opinion, just an observation.)

@bzbarsky
Copy link
Collaborator

That one is pretty inefficient, though.

@annevk
Copy link
Member

annevk commented May 30, 2018

The main reason we removed [ArrayClass] is that tagging it on things we wanted it for, e.g., NodeList, wasn't web-compatible. And for new lists we generally want folks to return arrays.

So therefore our options seem to be:

  1. Accept the inefficiency and slight inelegance of [...obj].find(callback) (and perhaps try to optimize it if it becomes a pattern).
  2. See if TC39 is interested is interested in a more elegant and efficient API for objects that have @@iterator. @domenic might know this?
  3. Test which Array properties we can add to IDL iterators without breaking the web and add them.

@marcoscaceres
Copy link
Member Author

marcoscaceres commented May 30, 2018

For 1, it's definitely already a pattern (with Array.from(), presumedly because Edge only just got full iterator support for NodeList): see sample query on github.

@domenic
Copy link
Member

domenic commented May 30, 2018

IMO if you want an array, you should not be using iterable<>. You should be using an array. iterable<> is when you need to create some completely custom collection, e.g. DOMTokenList (a type of set), URLSearchParams (a multi-map), or when you need to adapt legacy array-likes.

@annevk
Copy link
Member

annevk commented May 30, 2018

@domenic it's not clearly stated in OP, but the goal of this discussion is to improve NodeList and other "legacy" interfaces that have no way out and are returned aplenty.

@marcoscaceres
Copy link
Member Author

@annevk thanks for clarifying the intent.

I agree with @domenic that an iterable is not an Array. I'm also sympathetic that we can't get NodeList to extend Array, as it will probably break the web.

What is clear is that developers want an array-like - so maybe @bzbarsky's whack-a-mole solution (#561 (comment)) could work with maybe a [ArrayMethods] or some kind of mixin that has map(), filter(), reduce() etc.

@annevk
Copy link
Member

annevk commented May 31, 2018

As per #561 (comment) we'll have to test those properties individually as we know adding all of them breaks the web.

@marcoscaceres
Copy link
Member Author

Sure, agree. Is there precedent for how we go about testing this web compat?

@bzbarsky
Copy link
Collaborator

Was the we breakage from properties, or from instanceof Array?

@annevk
Copy link
Member

annevk commented May 31, 2018

I don't remember.

@marcoscaceres
Copy link
Member Author

Maybe something about NodeList being a live collection?

@bzbarsky
Copy link
Collaborator

I looked it up: the specific compat issue was people doing .concat() on nodelists because they tested true for instanceof Array. So nodelists that were ArrayClass and implemented @@isConcatSpreadable would not have had the one known compat issue.

The real issue was that no one except Gecko was interested in implementing [ArrayClass]...

@tabatkins
Copy link
Contributor

IMO if you want an array, you should not be using iterable<>. You should be using an array. iterable<> is when you need to create some completely custom collection, e.g. DOMTokenList (a type of set), URLSearchParams (a multi-map), or when you need to adapt legacy array-likes.

Or when you want something that looks like an Array but actually implements the same sort of typechecking that every single other object in WebIDL is allowed to implement...

@saschanaz
Copy link
Member

saschanaz commented Jul 23, 2018

I hope the ES array methods (edit: optionally) use Symbol.iterator rather than requiring length property. Doing that will at least ease writing a helper library without a need to rewrite everything.

@bzbarsky
Copy link
Collaborator

I hope the ES array methods (edit: optionally) use Symbol.iterator rather than requiring length property

Which methods?

Generally speaking, the Array generics in ES in fact use the length property; they were mostly designed before iterators existed and were meant to work on arbitrary arraylikes.

@saschanaz
Copy link
Member

saschanaz commented Jul 23, 2018

Which methods?

Nearly all, except push() and pop() which won't work with iterators, so that this can just work:

Array.prototype.find.call(iterableWithoutLength, fn); // does not creates a new array

@bzbarsky
Copy link
Collaborator

https://tc39.github.io/ecma262/#sec-array.prototype.find gets the .length and then walks from 0 to that length.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

7 participants