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

Accept non-iterator array-like inputs, like Array.from? #7

Closed
js-choi opened this issue Sep 15, 2021 · 23 comments
Closed

Accept non-iterator array-like inputs, like Array.from? #7

js-choi opened this issue Sep 15, 2021 · 23 comments
Labels
question Further information is requested

Comments

@js-choi
Copy link
Collaborator

js-choi commented Sep 15, 2021

Array.from accepts non-iterable array-like inputs (objects with a length property and indexed elements).

Should the inputs of Array.fromAsync be a superset of Array.from?

@ljharb
Copy link
Member

ljharb commented Sep 15, 2021

It absolutely should accept them.

@zloirock
Copy link
Contributor

zloirock commented Sep 15, 2021

Iterators helpers and collections .from does not handle non-iterables. However, it can be added for consistency with Array.from. But how? Array iterator -> async-from-sync iterator - and iteration over it, just iteration over array-like object or something else?

@js-choi
Copy link
Collaborator Author

js-choi commented Sep 15, 2021

I don’t think iterator helpers need to accept non-iterator array-like inputs. iter.toArray was never going to completely match Array.from, right?

@zloirock
Copy link
Contributor

(Async)Iterator.from, (Weak)(Map|Set).from.

@js-choi
Copy link
Collaborator Author

js-choi commented Sep 15, 2021

Oh, ah, right, I had forgotten about those. Well, yes, those might want to match Array.from in taking non-iterable array-like inputs. We should open an issue there.

@zloirock
Copy link
Contributor

zloirock commented Sep 15, 2021

I'm not sure that handling non-iterables should be added to all .from methods. Array-like is an obsolete protocol for collections that was replaced by the iterable protocol in ES6. Array-like objects are... array-like, so they could be transformed into arrays, they are sync, so I don't think that makes sense to handle them async - Array.from is enough. It's a legacy protocol, so I don't think that makes sense to add it to new features. Because of this, new features that accept collections support only iterables, but not array-like objects - for example, collections constructors like Set.

@ljharb
Copy link
Member

ljharb commented Sep 15, 2021

It's very much not obsolete, and it's very nice that things aren't forced to implement the iterator protocol to be transformable into an Array.

This proposal must match Array.from's semantics, which includes every feature it has - just like new array methods still include a receiver argument, despite that being obsolete.

@zloirock
Copy link
Contributor

zloirock commented Sep 15, 2021

It's very much not obsolete, and it's very nice that things aren't forced to implement the iterator protocol to be transformable into an Array.

How many new features are array-like and not iterable? The iterable protocol is the standard for handling collections. It's a bad practice not to use iterators protocol where it can be used.

It's not hard to implement iterators protocol on a userland array-like - just add [Symbol.iterator]: [].values.

just like new array methods still include a receiver argument, despite that being obsolete

It was decided not to add this argument to new features. It's added only to new completely similar to old features. For example, this argument was not added to (Async)Iterator.prototype methods similar to Array.prototype methods.

This proposal must match Array.from's semantics

It's like to say that new array prototype methods should not handle holes as undefined like old methods.

@ljharb
Copy link
Member

ljharb commented Sep 15, 2021

There's a difference - holes are bad and should never have been a thing, arraylikes are fine and have use cases beyond iteration.

@zloirock
Copy link
Contributor

Array-like objects are fine, but for interaction with modern infrastructure, they should be also iterable. For handling legacy non-iterable array-like objects already available sync Array.from, any other methods haven't fallbacks for this legacy.

@ljharb
Copy link
Member

ljharb commented Sep 15, 2021

I agree with you there. The sole purpose of these .from methods is to not have to think about the legacy stuff - in other words, you can always feed them legacy things, and you get proper, non-legacy things out.

@zloirock
Copy link
Contributor

Let it be. But let's return to this:

But how? Array iterator -> async-from-sync iterator - and iteration over it, just iteration over array-like object or something else?

@bakkot
Copy link
Contributor

bakkot commented Sep 15, 2021

I guess I'm OK with it, but I'm not totally convinced. fromAsync is useful specifically for async iterables. I have a hard time imagining why reasonable code would ever want to pass it an array-like (which is by definition synchronous).

@js-choi
Copy link
Collaborator Author

js-choi commented Sep 15, 2021

But how? Array iterator -> async-from-sync iterator - and iteration over it, just iteration over array-like object or something else?

Yeah, I think it would just return a promise that resolves to the array that would have been created by Array.from.


I guess I'm OK with it, but I'm not totally convinced. fromAsync is useful specifically for async iterables. I have a hard time imagining why reasonable code would ever want to pass it an array-like (which is by definition synchronous).

I imagine people will be occasionally passing sync iterables to fromAsync when they’re not sure whether it’s an async or sync iterable, just how they might be doing so with for await. That’s all I have for use cases. Separately, consistency is important.


Also, I created tc39/proposal-iterator-helpers#155.

@bakkot
Copy link
Contributor

bakkot commented Sep 15, 2021

I'm not at all convinced by the consistency argument. This is an async method, which is a fairly different beast. It is not obvious to me why it should accept all of the synchronous inputs that its synchronous sibling does.

@zloirock
Copy link
Contributor

zloirock commented Sep 15, 2021

Yeah, I think it would just return a promise that resolves to the array that would have been created by Array.from.

The problem in #4 + #6, in this case:

await Array.fromAsync([Promise.resolve(1), 2, Promise.resolve(3)]); // => [1, 2, 3]

await Array.fromAsync({ 0: Promise.resolve(1), 1: 2, 2: Promise.resolve(3), length: 3 }); // => [Promise.resolve(1), 2, Promise.resolve(3)]

@js-choi
Copy link
Collaborator Author

js-choi commented Sep 15, 2021

I'm not at all convinced by the consistency argument. This is an async method, which is a fairly different beast. It is not obvious to me why it should accept all of the synchronous inputs that its synchronous sibling does.

I don’t have any strong opinion either way. But it seems like there is significant disagreement within TC39 about this issue. What would be the best way to resolve this: keeping it in this issue, talking about it at plenary, an incubator call, or all of these?

@bakkot
Copy link
Contributor

bakkot commented Sep 15, 2021

Well, so far you've only heard from two committee members, so it's hard to say what the opinion of the committee as a whole would be. It's hard to get a broader sense of opinions without raising it in plenary, so that's probably what I'd do as the next step.

js-choi added a commit that referenced this issue Sep 18, 2021
js-choi added a commit that referenced this issue Sep 18, 2021
@js-choi js-choi added the question Further information is requested label Sep 19, 2021
js-choi added a commit that referenced this issue Sep 30, 2021
js-choi added a commit that referenced this issue Sep 30, 2021
js-choi added a commit that referenced this issue Sep 30, 2021
js-choi added a commit that referenced this issue Sep 30, 2021
js-choi added a commit that referenced this issue Sep 30, 2021
js-choi added a commit that referenced this issue Sep 30, 2021
@theScottyJam
Copy link

I've been playing around with the Gzemnid dataset for the top 1000 downloaded NPM packages (basically it's a large text file containing all of their source code), and have been trying to find relevant information for another proposal. @js-choi asked me to see if I could also find anything relevant to this thread from the dataset as well.

Searching the dataset for usages of Array.from, Array.prototype.slice.call, and [].slice.call brings me tons of results in which people are trying to transform an array-like into an array. Many of these array-like objects are the arguments object, and nodeLists, but there's other uses that show up as well.

To find anything relevant to this issue, I decided to look for anything that had Array.from/Array.prototype.slice.call/[].slice.call together with Promise.resolve on the same line, as that would precisely emulate the behavior of Array.fromAsync() if it supported array-like objects (at least, that's how I've understood it would behave from this conversation). I was able to find only one relevant result. It came from v0.0.1 of the promise-rate-limit package, within a test case.

test('passes through arguments', (t) => {
  const rateLimited = rateLimit(10, 100, resolveArguments);
  rateLimited('a', 'b', 'c').then((res) => {
    t.deepEqual(res, ['a', 'b', 'c']);
    t.end();
  });

  function resolveArguments() {
    return Promise.resolve([].slice.call(arguments));
  }
});

They had created a helper "resolveArguments()" function that took an arbitrary list of arguments, turned them into an array, then returned that array as a promise, because they were trying to stub an async callback. Today, that helper would probably be written simply like this: async (...args) => args.

I also tried checking for any scenario with Promise.resolve() was on the line next to the arrayLike-to-array operation, in case they split up the logic over multiple lines, but I didn't find anything relevant with that search.

Of course, it's very possible people that many others were trying to do this kind of logic, but they had simply split it up over many lines, like this:

const data = Array.from(...)
...irrelevant logic...
return Promise.resolve(data)

Or, it's possible that people were doing this sort of logic through other means that I wasn't searching for at all. If anyone else is interested in looking at some of this data themselves, I can share the first few thousand occurrences of Array.from/Array.prototype.***.call/[].***.call, or you can download the raw dataset yourself, using the instructions @js-choi shared on the bind proposal.

@theScottyJam
Copy link

theScottyJam commented Oct 12, 2021

Above is the raw data I could find, now I'll share a couple of my opinions on the matter. From what I can tell, it seems like turning an arrayLike into a promise isn't an extremely common operation, but I'm sure there's the occasional use for it (I just couldn't find any great examples from the dataset using the blunt, text-searching tools I was using). So, usability-wise, supporting ArrayLike with Array.fromAsync() probably isn't that important, especially since it's not that hard to just do Promise.resolve(Array.from(...)) and get the same effect. It could even be argued that the explicit Promise.resolve() version is more readable (this is of course, debatable).

So, it's probably best to do whatever feels the most consistent, and what you think programmers would expect.

I get the argument that Array.from() supports array-like, so it would be good to make Array.fromAsync() support it as well, but if the way to do that isn't an obvious, no-brainer "of course that's how fromAsync would behave with an array-like", and if it's unlikely to be a very useful feature, then perhaps it's better to just leave it out, even if it means there's a slight inconsistency in their behaviors.

Though, I'm a little torn. I don't know what developers would actually expect.

update: On second thought, I wonder if I'm been looking for the wrong thing. First, I took a closer look at the proposal, and saw that Promise.resolve(Array.from(arrayLike)) is not the same thing, as it'll also await each promise in the array-like. I can't think of any array-like structures off of the top of my head that would have promises (besides maybe the legacy arguments array), but there very well could be. Second, I'm realizing that the code author may want to just explicitly support whatever kind of array-like, sync/async iterable within their function, without having to detect what it is and have different code paths for each type. I'll see if I can come up with other ways to analyze the data, to look specifically for these scenarios.

@ljharb
Copy link
Member

ljharb commented Oct 12, 2021

An arraylike would be treated exactly the same as an array; the language primarily accepts an arraylike anywhere an array is accepted, except for places that only accept iterables.

@js-choi
Copy link
Collaborator Author

js-choi commented Oct 26, 2021

I made a brief update presentation about this issue to the Committee at the October plenary today. Although feedback time was but brief, I got no clear signals of opposition to making Array.fromAsync’s inputs be a superset of Array.from’s.

@syg asked whether each item from the array-like input would be awaited. I confirmed yes, since that’s what would happen with a non-async sync iterable (just like with for await; see #9).

I plan to move forward with allowing non-iterator array-like inputs when I present this proposal again for Stage 2 in a few months.

@js-choi
Copy link
Collaborator Author

js-choi commented Dec 14, 2021

Reached Stage 2 today, with the answer to this question as yes.

@js-choi js-choi closed this as completed Dec 14, 2021
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 15, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
question Further information is requested
Projects
None yet
Development

No branches or pull requests

5 participants