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

Use standard Promise.allSettled instead polyfill #4905

Merged
merged 1 commit into from
Aug 10, 2022

Conversation

outsideris
Copy link
Contributor

Description of the Change

We've introduced @ungap/promise-all-settled in #4476.
And Node.js support Promise.allSettled in Node 12.9.0.

So, we don't need polyfill anymore because we support Node 14+.

Why should this be in core?

We don't need polyfill anymore.

Benefits

dependency is reduced.

Possible Drawbacks

N/A

@outsideris outsideris added the type: chore generally involving deps, tooling, configuration, etc. label Jul 31, 2022
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.002%) to 94.326% when pulling c557d79 on use-builtin-allsettled into 84b2f84 on master.

@juergba
Copy link
Contributor

juergba commented Aug 9, 2022

@outsideris I guess this PR makes sense.
One of your links points to a korean page. I couldn't find any docs about Node supporting Promise.allSettled(). Can you help, please?

@outsideris
Copy link
Contributor Author

Copy link
Contributor

@juergba juergba left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@outsideris thank you for your explanations and this PR.

@juergba juergba merged commit 77c18d2 into master Aug 10, 2022
@juergba juergba added this to the next milestone Aug 10, 2022
@juergba juergba added the type: cleanup a refactor label Aug 10, 2022
This was referenced Aug 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: chore generally involving deps, tooling, configuration, etc. type: cleanup a refactor
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants