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

Add iterable support #1

Open
aomega opened this issue Jun 27, 2016 · 5 comments
Open

Add iterable support #1

aomega opened this issue Jun 27, 2016 · 5 comments

Comments

@aomega
Copy link

aomega commented Jun 27, 2016

Will this support the equivalent of Promises.all(...)?

e.g.

pipeP((x, y) => [Promise.resolve(x), Promises.resolve(y)], ([x, y]) => x + y)

i.e. where the two promises are resolved async?

@zspecza
Copy link
Owner

zspecza commented Jun 28, 2016

Hi @aomega - Yes, it's all just promises under the hood so it should resolve asynchronously. This pipeP implementation will automatically call Promise.all for you any time it encounters an array. Take the following code, for example - I've used Node's process.hrtime builtin to record how long it takes to resolve an addition like in your example:

(You can see the code work here)

const pipeP = require("pipep")
const delay = (ms) => new Promise((resolve) => setTimeout(resolve, ms))

const add = pipeP(
    (a, b) => {
        return [
            delay(4000).then(() => a),
            delay(3000).then(() => b)
        ]
    },
    ([a, b]) => a + b
)

const start = process.hrtime()

add(10, 5).then((val) => {
  const end = process.hrtime(start)
  console.log(`got ${val} after ${end.toString()} seconds`)
})

For me, the output averages at around 4.2 seconds - which means the array with the promises in the initial handler has the same behavior as if they were called with Promise.all :)

@aomega
Copy link
Author

aomega commented Jun 28, 2016

I was also wondering if this could be adapted to support iterables rather than just arrays. Then it would be possible to use it with libraries like Immutable (without that being a dependency). I might experiment. We use Immutable a lot and I was looking for a way to resolve promises inside immutable structures - looks like this would be a starting point. Great work!

@zspecza
Copy link
Owner

zspecza commented Jun 29, 2016

Hmm, that's a good idea.

I've updated the title of this issue to reflect this.

It shouldn't be too big of a change, since Promise.all already accepts an iterable and will automatically coerce it to an array for us when it resolves - I think all that needs to be done to support this is to add another else if statement here that checks if val is an iterable. I'll look into it when I get some time if you don't get the chance to experiment, but I'll have to check around to see how iterables can be properly cloned :)

@zspecza zspecza changed the title Async promises? Add iterable support Jun 29, 2016
@aomega
Copy link
Author

aomega commented Jun 29, 2016

I created a simple experiment to try this [https://gist.github.com/aomega/9723e628746040c2917f28dc8a20e41b]

The file would run with node if you put it in the test/ directory of this project and install 'immutable' and 'transducers.js'.

The function goes further than resolving iterables - it also resolves plain objects. But more importantly it can build the original structure as the result.

In the experiment, it uses transducers.js to build the result. However, I could see it would be relatively easy to extract the building logic so as not to have a dependency on transducers.js. The only thing needed would be for the datastructures to support the 'transducers' interface to enable the structures to be built after promise resolution (I added them here to Immutable Map and List, but there is a MR to add them to Immutable.js mainline branch).

It also works as a handler in pipeP so it could be used in conjunction with pipeP, rather than giving the capability to pipeP, perhaps.

@zspecza
Copy link
Owner

zspecza commented Jun 30, 2016

@aomega interesting - I don't think we'd need transducers or even the processCollection function - as long as we stick to the iterable spec, we should be able to get away with just the native Promise.all iterable behavior. I'll have to do some experimenting of my own to confirm this - but I'll have to do it over the weekend as I'm rather swamped at the moment

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

No branches or pull requests

2 participants