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

Maybe async refactor #1287

Merged
merged 4 commits into from
Sep 30, 2022
Merged

Maybe async refactor #1287

merged 4 commits into from
Sep 30, 2022

Conversation

nerfZael
Copy link
Contributor

No description provided.

@nerfZael nerfZael changed the base branch from origin to origin-dev September 28, 2022 00:17
@lgtm-com
Copy link

lgtm-com bot commented Sep 28, 2022

This pull request fixes 1 alert when merging 0916400 into b95c714 - view on LGTM.com

fixed alerts:

  • 1 for Useless assignment to local variable

@nerfZael nerfZael marked this pull request as ready for review September 28, 2022 13:04
Copy link
Contributor

@pileks pileks left a comment

Choose a reason for hiding this comment

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

LGTM - We should probably dig a little into the TS spec to see what exactly allows us to await a union of T and Promise<T>, but if it fits I sits.

Copy link
Contributor

@cbrzn cbrzn left a comment

Choose a reason for hiding this comment

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

🥇 looks good!

@@ -1,18 +1 @@
export type MaybeAsync<T> = Promise<T> | T;
Copy link
Contributor

Choose a reason for hiding this comment

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

oh wow, had no clue you could await a non-async function and it'd return just fine.

@dOrgJelli dOrgJelli merged commit 826b192 into origin-dev Sep 30, 2022
@dOrgJelli dOrgJelli deleted the nerfzael-maybe-async-refactor branch April 10, 2023 17:04
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.

4 participants