Skip to content

Conversation

@dimaMachina
Copy link
Collaborator

why for non-React - because it's ugly have async IIFE inside useEffect

@changeset-bot
Copy link

changeset-bot bot commented Mar 31, 2023

🦋 Changeset detected

Latest commit: f8d1232

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 9 packages
Name Type
@graphiql/toolkit Patch
graphql-language-service-server Patch
monaco-graphql Patch
vscode-graphql Patch
vscode-graphql-execution Patch
@graphiql/react Patch
graphiql Patch
graphql-language-service-cli Patch
@graphiql/plugin-explorer Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

Comment on lines 70 to 71
iteratorReturn?.();
return result.value;
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

the only thing is that after refactoring to async/await we call iteratorReturn?.() before returning values, previously we did following, does it ok?

        resolve(result.value);
        // ensure cleanup
        iteratorReturn?.();

};

this.loadProvider()
.then()
Copy link
Collaborator Author

@dimaMachina dimaMachina Mar 31, 2023

Choose a reason for hiding this comment

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

I guess call .then is doing nothing

@dimaMachina dimaMachina marked this pull request as ready for review April 1, 2023 03:05
Copy link
Contributor

@TallTed TallTed left a comment

Choose a reason for hiding this comment

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

Minor tweak of comment for clarity

@dimaMachina dimaMachina force-pushed the promise/prefer-await-to-then branch from 91333f2 to f8d1232 Compare April 14, 2023 18:45
@dimaMachina dimaMachina merged commit 15c26eb into graphql:main Apr 14, 2023
@dimaMachina dimaMachina deleted the promise/prefer-await-to-then branch April 14, 2023 18:49
@acao acao mentioned this pull request Apr 14, 2023
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.

3 participants