Skip to content

RFC: Synchronous execution #1115

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

Merged
merged 3 commits into from
Dec 5, 2017
Merged

RFC: Synchronous execution #1115

merged 3 commits into from
Dec 5, 2017

Conversation

leebyron
Copy link
Contributor

@leebyron leebyron commented Dec 5, 2017

This is a breaking change. Existing uses of execute() outside of async functions which assume a promise response will need to wrap in Promise.resolve()

This allows execute() to return synchronously if all fields it encounters have synchronous resolvers. Notable this is the case for client-side introspection, querying from a cache, and other useful cases.

Also introduces graphqlSync() as a top level function which asserts sync behavior and includes validation.

Note that the top level graphql() function remains a Promise to minimize the breaking change, and that validate() has always been synchronous.

**This is a breaking change. Existing uses of execute() outside of async functions which assume a promise response will need to wrap in Promise.resolve()**

This allows `execute()` to return synchronously if all fields it encounters have synchronous resolvers. Notable this is the case for client-side introspection, querying from a cache, and other useful cases.

Note that the top level `graphql()` function remains a Promise to minimize the breaking change, and that `validate()` has always been synchronous.
@IvanGoncharov
Copy link
Member

@IvanGoncharov I'd like your opinion of if this will cover the more general cases you've encountered.

@leebyron Looks great 👍 and solves all the cases I encounter. One less promise to worry about 😄
I've opened #1118 and #1120 for the small suggestions.

@leebyron leebyron mentioned this pull request Dec 6, 2017
martijnwalraven added a commit to apollographql/apollo-server that referenced this pull request Dec 18, 2017
This fixes executing a query if all fields it encounters have synchronous resolvers, for which behavior changed in 0.12.x due to graphql/graphql-js#1115.

Closes #718, closes #720
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants