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 ! with a PromiseResolve call that can't fail #216

Merged
merged 2 commits into from Aug 26, 2022
Merged

Conversation

bakkot
Copy link
Collaborator

@bakkot bakkot commented Aug 10, 2022

A slight tweak to #215.

The reason to use PromiseResolve is to get the fast-path when the value being passed is expected to be a Promise. On this particular line we've just created the iterator result object with which we are resolving, so we know the value is not a Promise. This is equivalent but (I think) clearer.

If this isn't accepted, the ? in the Return ? PromiseResolve should at least be changed to !, since the throwy case is guarded on IsPromise of the argument.

@michaelficarra
Copy link
Member

I think I'd rather leave PromiseResolve and just use the !.

Copy link
Member

@michaelficarra michaelficarra left a comment

Choose a reason for hiding this comment

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

Updated. Merging.

@michaelficarra michaelficarra changed the title avoid using PromiseResolve with a value known not to be a Promise use ! with a PromiseResolve call that can't fail Aug 26, 2022
@michaelficarra michaelficarra merged commit 96bc898 into main Aug 26, 2022
@ljharb ljharb deleted the less-resolve branch August 26, 2022 18:53
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