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

Promise usage #10

Closed
benjamingr opened this issue Sep 3, 2017 · 4 comments
Closed

Promise usage #10

benjamingr opened this issue Sep 3, 2017 · 4 comments

Comments

@benjamingr
Copy link
Contributor

Hi, thanks for the nice library!

In the code you create a resolvable() only to resolve it with an already existing promise which is not something you typically have to do.

You can use fetch as this.response, the setTimeout also isn't necessary as promises take care of reacting on the next tick anyway.

I'll write a PR to illustrate what I mean

@bsouthga
Copy link

bsouthga commented Sep 3, 2017

I'm not sure about the original intent with the resolver pattern, but promises actually don't resolve on the next event loop turn -- they resolve on the next microtask: https://stackoverflow.com/questions/25915634/difference-between-microtask-and-macrotask-within-an-event-loop-context.

You can see this clearly by running the following code in node or the browser:

Promise.resolve().then(() => console.log(1));
setTimeout(() => console.log(2), 0);
Promise.resolve().then(() => console.log(3));
console.log(4);

Which should output

4
1
3
2

(edited to fix for extra .resolve())

@benjamingr
Copy link
Contributor Author

I actually started with that (promisifying setTimeout and using a setTimeout(fn, 0)) but then I realized it was only deferred so things like setting options would work (the fetch must be dispatched after all the configuration is done - meaning waiting a microtick is plenty.

That said, neither Node.js nor the browsers actually makes any guarantees about the order of things, and it's possible for promise callbacks to run before or after timers. Node.js explicitly makes no such guarantee about scheduling - only best attempt.

@benjamingr
Copy link
Contributor Author

Here's a reference btw nodejs/node#15081

@mikeal
Copy link
Owner

mikeal commented Apr 6, 2018

🎉 This issue has been resolved in version 2.0.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

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

No branches or pull requests

3 participants