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

Do we want to expose AsyncFromSyncIterator? #172

Closed
Jamesernator opened this issue Jan 29, 2022 · 6 comments · Fixed by #182
Closed

Do we want to expose AsyncFromSyncIterator? #172

Jamesernator opened this issue Jan 29, 2022 · 6 comments · Fixed by #182

Comments

@Jamesernator
Copy link

Currently AsyncFromSyncIterator exists pretty much entirely as spec fiction to enable sync iterators to be consumed by for-await-of and the like. However in the current spec text for the proposal the (probably intended) of AsyncIterator.from can return real AsyncFromSyncIterator objects (and hence AsyncFromSyncIteratorPrototype could be manipulated).

Is this something we actually want to do however? At current by being just spec fiction, engines can presumably skip creation of such things entirely and go down short paths, however because of the way the iterator protocol works if AsyncFromSyncIterator objects are exposed, and consequently tampered with by manipulating their methods, such assumptions may be consequently broken.

The simple way to avoid exposing them is just to wrap the AsyncFromSyncIterator itself again in AsyncIterator.from, although I'm unsure if this double wrapping itself could expose things.

If on the other hand we do want to expose AsyncFromSyncIterator for realsies do we actually want them returned from AsyncIterator.from? Currently no ECMA262 objects return concrete subclasses for these sort've methods*, although web APIs and various userland APIs do so it's not really unprecendented.

* NOTE that TypedArray methods are accessed by their subclasses, in fact TypedArray isn't even exposed as a named global and requires looking up the prototype chain to even find.

@bakkot
Copy link
Collaborator

bakkot commented Jan 29, 2022

We definitely should not make it possible to break the built-in for await (let x of syncIterable) logic by monkey-patching AsyncFromSyncIteratorPrototype.

@ljharb
Copy link
Member

ljharb commented Jan 29, 2022

It seems like it should be trivial enough to make for-await-of call into intrinsics instead of doing observable lookups on a prototype?

@mhofman
Copy link
Member

mhofman commented Jan 30, 2022

Thanks for raising this. If the intent is to expose AsyncFromSyncIterator, this proposal should make it explicit, and the prototype should be discoverable by walking the globals somehow, not requiring a call to AsyncIterator.from with a special value. This intrinsic discoverability will be a blocker.

@ljharb, the problem is that the spec is written to handle an iterator record, regardless of what the iterator is, and lookup methods on it. While you may be able to update CreateAsyncFromSyncIterator to set [[NextMethod]] to the intrinsic instead of looking it up, it'd be a breaking change to capture the "return" method similarly. I suppose all algorithms using iterator records could be updated to handle an optional [[ReturnMethod]] on the record that would only be populated by CreateAsyncFromSyncIterator, but that seems like a large change.

I'd personally prefer we find a way not to expose this previously spec internal object.

@ljharb
Copy link
Member

ljharb commented Jan 30, 2022

I don’t see how exposing it is avoidable.

@devsnek
Copy link
Member

devsnek commented Jan 30, 2022

i think the point here is more that for-await should be hardened than that we should try to hide AsyncFromSyncIterator. since AsyncFromSyncIterator doesn't technically exist yet we have a bit of creative freedom in how we specify AsyncIterator.from and for-await to handle these cases how we like.

@mhofman
Copy link
Member

mhofman commented Jan 30, 2022

I don’t see how exposing it is avoidable.

Since AsyncIterator.from wraps some other iterators, it could very well wrap this spec internal iterator as well?

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