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

Always run limited functions asynchronously #28

Conversation

copperwall
Copy link
Contributor

This modifies the generator function returned from plimit to always
run functions passed to limit asynchronously.

This is done by modifying the enqueue method to always push the function
provided to the generator function onto the queue and then scheduling a
function to run asynchronously to dequeue and run that function.

I thought using using Promise.resolve().then() was a nice way to
schedule that dequeuing function asynchronously because it wasn't too
node-specific or browser-specific, but I'm happy to do something else
like preferring process.nextTick and then falling back to setTimeout
if process.nextTick isn't available.

Closes #22

This modifies the generator function returned from plimit to always
run functions passed to limit asynchronously.

This is done by modifying the enqueue method to always push the function
provided to the generator function onto the queue and then scheduling a
function to run asynchronously to dequeue and run that function.

I thought using using `Promise.resolve().then()` was a nice way to
schedule that dequeuing function asynchronously because it wasn't too
node-specific or browser-specific, but I'm happy to do something else
like preferring `process.nextTick` and then falling back to `setTimeout`
if `process.nextTick` isn't available.
index.js Outdated Show resolved Hide resolved
@sindresorhus
Copy link
Owner

I thought using using Promise.resolve().then() was a nice way to
schedule that dequeuing function asynchronously because it wasn't too
node-specific or browser-specific, but I'm happy to do something else
like preferring process.nextTick and then falling back to setTimeout
if process.nextTick isn't available.

It really depends on how much breathing room we want to give it. setImmediate runs in the next even loop tick. nextTick run at the end of the current event loop tick. And Promise.resolve() runs in the next macro-tick (which is much shorter). Since we don't know the use-case, I think Promise.resolve() is fine and it also works in any environment.

@copperwall
Copy link
Contributor Author

Hey @sindresorhus, it looks like the node 6 test failed from using async. I know node 6 and 8 are EOL'd, but I don't want to break compatibility if this library is still targeting those versions. I can revert the change or bump the engine requirement, either way.

@sindresorhus
Copy link
Owner

I plan to target Node.js 10 after this PR, so just ignore anything lower.

index.js Show resolved Hide resolved
@sindresorhus sindresorhus changed the title Always run limit fns asynchronously Always run limited functions asynchronously May 18, 2020
@sindresorhus sindresorhus merged commit 7b978e3 into sindresorhus:master May 18, 2020
@sindresorhus
Copy link
Owner

Thanks for fixing this 🙌

@sindresorhus
Copy link
Owner

This is now published: https://github.com/sindresorhus/p-limit/releases/tag/v3.0.0

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

Successfully merging this pull request may close these issues.

Function sometimes run synchronously, sometimes asynchronously
2 participants