Skip to content
This repository has been archived by the owner on Jan 26, 2022. It is now read-only.

Don't fallback to Symbol.iterator in async iteration statement #12

Closed
zenparsing opened this issue Jul 24, 2015 · 27 comments
Closed

Don't fallback to Symbol.iterator in async iteration statement #12

zenparsing opened this issue Jul 24, 2015 · 27 comments

Comments

@zenparsing
Copy link
Member

Looking over this again, the behavior where we fallback to using Symbol.iterator if the iteration subject does not have Symbol.asyncIterator feels hacky.

I think it should only look for Symbol.asyncIterator. If we want to, we can add async iteration support to regular iterators by adding a method to `%IteratorPrototype%.

@RangerMauve
Copy link

I think adding Symbol.asyncIterator to the IteratorPrototype would be absolutely necessary if regular iteraters won't work. Also, what if it's a regular iterator that yields promises?

@zenparsing
Copy link
Member Author

Also, what if it's a regular iterator that yields promises?

Then it's not a regular iterator ; )

A regular (synchronous) iterator must return IterationResult objects. An async iterator must return Promise<IterationResult> objects.

@RangerMauve
Copy link

Ah! That makes sense.

@RangerMauve
Copy link

What does the async function spec say about using await on non-promises? If it errors out then I think that it would make sense for for async to error out if one tries to iterate over something that isn't an async iterator

@zenparsing
Copy link
Member Author

@RangerMauve awaiting a non-promise is equivalent to awaiting a promise which is immediately fulfilled with that value.

@RangerMauve
Copy link

I would argue, then, that if something other than an asyc iterator gets passed in, it should be treated as a regular iterator, but maybe the actual iteration should use microtasks so that it isn't sync.

@zenparsing
Copy link
Member Author

Wow, it's been a while since that last comment, but @RangerMauve I think you're right.

@RangerMauve
Copy link

:D Woot!

@groundwater
Copy link

I have been working with async iterators in my last project, so I thought I'd add my thoughts.

I believe the power of async iterators comes from composing them into larger computations. Each step may do some I/O, for example the first iterator may yield account ids which the second iterator then takes and returns the corresponding row form a database.

I have run into cases where a step of the computation can be 100% synchronous, e.g. performing some mathematical transform on values. I'd rather write the synchronous code within the iterator, but from the outside I don't want to give it special treatment.

Thus I'd agree with the above. It should be fine to pass in synchronous iterator, but they should behave like the asynchronous ones from the consumer perspective.

@zenparsing
Copy link
Member Author

All agreed, so closing : )

@zenparsing
Copy link
Member Author

Reopening due to some concern raised at TC39 about falling back to Symbol.iterator.

@zenparsing zenparsing reopened this Jan 29, 2016
@RangerMauve
Copy link

What was the concern specifically?

@zenparsing
Copy link
Member Author

About fallback in general, where adding a method to an existing type would change behavior in for-await statements. I just wanted to re-open so that I didn't forget to come back to this later.

@benlesh
Copy link

benlesh commented Apr 14, 2016

Ha! this looks like a duplicate of the issue I just opened #26. I too want a Symbol.asyncIterator but for different reasons.

@zenparsing
Copy link
Member Author

@Blesh There will definitely be a Symbol.asyncIterator. The question in this thread was whether for-await should fall back to using Symbol.iterator if it can't find a Symbol.asyncIterator method.

@azu
Copy link

azu commented Apr 21, 2016

@Jamesernator
Copy link

Jamesernator commented Jun 17, 2016

I think it should fallback to Symbol.iterator given that we can await either a synchronous or an asynchronous value, it makes sense that a for await would do the same

Take for example the readLines function from the examples:

async function *readLines(path) {
    let file = await fileOpen(path);
    try {
        while (!file.EOF)
            yield file.readLine();
    } finally {
        await file.close();
    }
}

for await (let line of readLines(path) {
    console.log(line);
}

should it actually matter if we replace readLines with the synchronous function:

function *readLines(path) {
    let file = fileOpenSync(path);
    try {
        while (!file.EOF)
            yield file.readline();
    } finally {
        file.close();
    }
}

I would say no for the same reason that we allow for awaiting a synchronous value in an asynchronous function, which is essentially that awaiting a synchronous value is not going to change the meaning of a program.

This is because we can just treat every synchronous value as an asynchronous value that has already resolved, in a similar vein we should be able to treat an Iterator as a AsyncIterator that will immediately resolve its values.

@eggers
Copy link

eggers commented Jun 17, 2016

How would behavior be different with fallback vs Symbol.asyncIterator added to standard iterator prototype?

@zenparsing
Copy link
Member Author

@eggers Good question.

Once difference would be that anyone wanting to manually implement the Iterator interface (without subclassing the standard iterator) would have to manually implement both Symbol.iterator and Symbol.asyncIterator, which might be annoying.

@Jamesernator
Copy link

Jamesernator commented Sep 12, 2016

A point I just thought about is that yield* will produce values from a Synchronous Iterator (as per #2), it would seem weird for

yield* syncIterator()

to yield values differently to this

for await (let val of syncIterator()) {
    yield val;
}

@littledan
Copy link
Member

Do we actually want to make this change? I thought it was resolved in committee that there should be the fallback.

@domenic
Copy link
Member

domenic commented Sep 12, 2016

The meeting notes seem unclear :( All I see are

WH: Another option already mentioned was to have for await only look for Symbol.asynciterator and not default to Symbol.iterator. This would require explicit user conversions to convert sequences to async sequences, but those aren't necessarily a bad thing.

DH: "meh".

I am inclined toward keeping the fallback, although I think it's an interesting idea to move the fallback to Iterator.prototype instead of making it part of the syntax.

@zenparsing
Copy link
Member Author

I actually don't recall a consensus being developed on this issue.

I think it's an interesting idea to move the fallback to Iterator.prototype instead of making it part of the syntax.

The only trouble there is that a user manually implementing an iterator without inheriting from Iterator.prototype would have to explicitly implement Symbol.asyncIterator themselves.

@domenic
Copy link
Member

domenic commented Sep 12, 2016

Yep, as you noted above. I just am not sure that's really a big deal, especially since it's an easy implementation.

@littledan
Copy link
Member

I think we should fall back to Symbol.iterator because our current Promise semantics are all about allowing sync things to be used as async things. You might call this "sloppiness". It follows @groundwater 's logic above, but I just want to spell out the parallels in more detail.

The "chaining" semantics of .then are all about this. You can return a Promise from .then or a scalar value; it's all the same. You call Promise.resolve not to wrap something in a Promise, but to cast something to a Promise--get an asynchronous value when you have something-or-other.

The semantics of async and await are all about being sloppy as well. You can slap await on any non-Promise expression in an async function and everything works fine, exactly the same way, except that you yield control to the job queue. Similarly, you can "defensively" put async around whatever you want, as long as you await the result. If you have a function that returns a Promise--whatever! you can make that an async function, and, from a user perspective, nothing changes (even if, technically, you get a different Promise object out).

Async iterators and generators should work the same way. Just like you can await a value that, accidentally, wasn't a Promise, a reasonable user would expect to be able to yield* a sync iterator within an async generator. for await loops should similarly "just work" if a user defensively marks a loop that way, thinking that they maybe might be getting an async iterator.

I think it would be a big deal to break all of these parallels. It would make async iterators less ergonomic. Let's discuss this the next time async generators/iterators come up on the agenda at TC39.

@domenic
Copy link
Member

domenic commented Sep 12, 2016

I agree with your reasoning, for sure. I think we should make sure that this is encoded in the spec, close this issue, and argue for it in TC39.

So the todos are that we need to make sure that not only for-await, but also yield*, have these fallback semantics. To me, again, the simplest way to do that is to have the fallback on Iterator.prototype, but I don't feel strongly.

@domenic
Copy link
Member

domenic commented Sep 16, 2016

Confirmed that this is handled in the spec via the modifications to GetIterator. They are pretty elegant and centralized so I am happy to go with that instead of Iterator.prototype fallback. Let's close this.

@domenic domenic closed this as completed Sep 16, 2016
domenic added a commit that referenced this issue Sep 26, 2016
This fixes the problem discussed in #15 (comment), for both for-await and yield*. It also changes the mechanism by which sync iterator adaptation happens from earlier drafts; before a996238, the adaptation happened in for-await-of by adding an additional await at each step, whereas now, we create an explicit "Async-from-Sync Iterator" adapter. Previously this was discussed around #12 (comment), although there we decided against this option; the recent movements around where unwrapping is performed have made it a better option.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

9 participants