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

refactor: remove superfluous 'async' from Promise executor #12468

Merged
merged 1 commit into from
May 21, 2024

Conversation

kkuegler
Copy link
Contributor

What kind of change does this PR introduce?

a (potential) bugfix

Did you add tests for your changes?

no, I deemed it straightforward enough

If relevant, did you update the documentation?

n/a

Summary

This 'async' is not needed here, because there is no 'await' in its body.

Having an async function as the executor for 'new Promise' is suboptimal, as it swallows synchronous errors that might be thrown in the body.

See https://stackoverflow.com/a/43050114/3394495

Does this PR introduce a breaking change?

no

Other information
n/a

Copy link

google-cla bot commented May 20, 2024

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@kkuegler kkuegler changed the title Remove superfluous 'async' from promsie executor Remove superfluous 'async' from promise executor May 20, 2024
Copy link

🤖 I detect that the PR title and the commit message differ and there's only one commit. To use the PR title for the commit history, you can use Github's automerge feature with squashing, or use automerge label. Good luck human!

-- conventional-commit-lint bot
https://conventionalcommits.org/

@kkuegler kkuegler changed the title Remove superfluous 'async' from promise executor refactor: Remove superfluous 'async' from Promise executor May 20, 2024
@Lightning00Blade Lightning00Blade self-requested a review May 21, 2024 07:34
This 'async' is not needed here, because there is no 'await'
in its body.

Having an async function as the executor for 'new Promise'
is suboptimal, as it may swallow synchronous errors that might
be thrown in the body.

See https://stackoverflow.com/a/43050114/3394495
@Lightning00Blade Lightning00Blade changed the title refactor: Remove superfluous 'async' from Promise executor refactor: remove superfluous 'async' from Promise executor May 21, 2024
@Lightning00Blade Lightning00Blade merged commit 1ad9aad into puppeteer:main May 21, 2024
19 checks passed
@kkuegler kkuegler deleted the patch-1 branch May 21, 2024 11:30
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.

None yet

2 participants