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

pipeAsync should be able to accept Promise as input #178

Closed
vitaly-t opened this issue Nov 1, 2022 · 4 comments
Closed

pipeAsync should be able to accept Promise as input #178

vitaly-t opened this issue Nov 1, 2022 · 4 comments
Assignees
Labels
enhancement New feature or request

Comments

@vitaly-t
Copy link
Owner

vitaly-t commented Nov 1, 2022

At the moment, we have to use toIterable when input is a promise. But for pipeAsync there is no reason not to do so automatically.

Function pipeAsync should allow Promise as input, and automatically call toIterable (internally):

import {pipeAsync, flat} from 'iter-ops';

const p = Promise.resolve([1, 2, 3, 4, 5]);

const i = pipeAsync(p, flat()); // this should just work, without using toIterable(p)

(async function () {
    for await (const a of i) {
        console.log(a); // 1, 2, 3, 4, 5
    }
})();

@RebeccaStevens More fun, if you are interested ;)

@vitaly-t vitaly-t added the enhancement New feature or request label Nov 1, 2022
@RebeccaStevens
Copy link
Collaborator

So just took a look at this and it seems really easy to do.
Just a few details we need to sort on how it should work:

  • Should it allow any promise or just promises that return an iterable (either sync or async)?
    • Allowing all promise types could be added in as single line (excluding typing updates).
      • Doing so would allow for single values to be used as iterable when returned by promises, but not if they aren't wrapped. If we go with this approach, we'd probably want to allow sync single values as well which would mean we should also make the change to sync pipeline as well.
    • Only allowing promises that return an iterable.
      • This wouldn't be hard either, but it would require some extra checks to be added to test the promise (no more than what toIterable is already doing).

@vitaly-t
Copy link
Owner Author

vitaly-t commented Nov 7, 2022

The result of pipeAsync(promise) should be consistent with pipe(toIterable(promise)). If we decide to extend toIterable later, it would be another matter. For now, we should just use the existing toIterable to provide consistent result.

@RebeccaStevens ;)

@RebeccaStevens
Copy link
Collaborator

What I'm getting at is the current toIterable supports more than just making promises iterable. It will also make single values into an iterable of one value.

If we simply use toIterable, we will be getting promise support but also other stuff; this would then make pipeAsync inconsistent with pipeSync.
Should pipeSync also get support for these other things or would it be better to make pipeAsync not also get that other stuff?

@vitaly-t
Copy link
Owner Author

vitaly-t commented Nov 8, 2022

You are right, there would be inconsistency, I forgot about simple values.

I think it is better then to close this issue, leave functionality as is, we do not want to create a discrepancy here.

@vitaly-t vitaly-t closed this as completed Nov 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants