search

rescript-lang / rescript-core

A drop-in standard library for ReScript. Intended to be familiar for JavaScript developers, easy to use, and be rich enough (without being bloated) so that you don't need to reach for anything else for typical ReScript development.
MIT License
163 stars 29 forks source link

Add make and convenience functions for async iterators #243

Closed zth closed 3 months ago

zth commented 3 months ago

This adds a make function and value + done convenience functions for async iterators.

cc @cometkim

zth commented 3 months ago

Curious about feedback from @cometkim

cometkim commented 3 months ago

I expect the producer and consumer of an iterator to work cooperatively.

However, in this example, only the producer can decide to stop, or it must expose its internal state to the outside world. This negates most of the benefits of the abstraction and is also of little use in FFI.

This is why @JonoPrest and I extended the topic in Retreat to include break/continue and generators.

https://github.com/JonoPrest/iterator-rescript/blob/retreat-note/RETREAT_NOTE.md

zth commented 3 months ago

@cometkim sure, and when that lands that'll be a much better solution. This a little something up until the point that it does land.

cometkim commented 3 months ago

I'd like to know the motivation and use case for adding this. I'm not sure if this would be useful without break/continue.

But it's easy to support if we add a small runtime for it. e.g. https://github.com/JonoPrest/iterator-rescript/blob/retreat-note/src/Demo.res

zth commented 3 months ago

I'd like to know the motivation and use case for adding this. I'm not sure if this would be useful without break/continue.

But it's easy to support if we add a small runtime for it. e.g. https://github.com/JonoPrest/iterator-rescript/blob/retreat-note/src/Demo.res

There are places where you need to return an actual AsyncIterator. One example is graphql-js subscriptions.

cometkim commented 3 months ago

Yes. The binding to iterator types and supporting a factory will be useful for FFIs. I had thought that forEach was newly added here.

I think adding a factory is ok, If the AsyncIterator.t type privides a proper forward compatibility.

zth commented 3 months ago

Yes. The binding to iterator types and supporting a factory will be useful for FFIs. I had thought that forEach was newly added here.

I think adding a factory is ok, If the AsyncIterator.t type privides a proper forward compatibility.

I guess it would, no?

zth commented 3 months ago

@cometkim in your eyes, can we merge this or are there still outstanding questions you want answers to?

cometkim commented 3 months ago

Yes. The binding to iterator types and supporting a factory will be useful for FFIs. I had thought that forEach was newly added here. I think adding a factory is ok, If the AsyncIterator.t type privides a proper forward compatibility.

I guess it would, no?

I think it would, or it can be easily migrated.

cometkim commented 3 months ago

forEach doesn't seem useful, but that's not related to this PR. Right?

zth commented 3 months ago

forEach doesn't seem useful, but that's not related to this PR. Right?

Not related to this PR, no. Why is it not useful though? I use it myself in a few places.

cometkim commented 3 months ago

forEach doesn't seem useful, but that's not related to this PR. Right?

Not related to this PR, no. Why is it not useful though? I use it myself in a few places.

As already mentioned, if the iterable is not array-like, its usability is limited without control flow support.

Or it can be (more) useful when combined with additional utilities like take(limit).

https://github.com/tc39/proposal-iterator-helpers/?tab=readme-ov-file#takelimit

zth commented 3 months ago

Right, got it!