Skip to content
This repository has been archived by the owner on Mar 20, 2023. It is now read-only.

Allow to replace default execute function #391

Merged
merged 1 commit into from Apr 7, 2019

Conversation

IvanGoncharov
Copy link
Member

This PR allows providing your own execute function instead of the default one from the graphql-js.
It’s a small change that adds a lot of flexibility since you can wrap standard execute and it opens new possibilities for middlewares.

Personally, I need it to proxy calls to 3rd-party APIs.
It’s an alternative to #253 but introduces less code and more flexible mechanism.
It’s very important for us as it will be the last change required to switch graphql-faker from custom forks of graphql-js and graphql-express.
@wincent @leebyron Can you please review it?

@matthewerwin
Copy link

@IvanGoncharov why has the signature been changed to a single args object? Public API that you cited on my PR http://graphql.org/graphql-js/execution/#execute doesn't take a single object argument so this seems like this code isn't working when the override execute isn't passed in.

@IvanGoncharov
Copy link
Member Author

IvanGoncharov commented Sep 16, 2017

@matthewerwin execute was extended to also support object as argument see here:
graphql/graphql-js#867
But no one updated the official docs 😞

Also, my PR to exposing ExecutionArgs type was merged: graphql/graphql-js#988
So it's easy to do type checks in Flow and TS.

I think receiving/returning objects is better than 7 separate values.

@matthewerwin
Copy link

@IvanGoncharov right on. Maybe make the update to the docs on that part of this PR. Thanks for the comprehensive info

@matthewerwin
Copy link

@IvanGoncharov I think before this PR gets merged it would be great to add two things:

  1. The documentation updates.
  2. change "response.statusCode = 400;" to "response.statusCode = contextError.status||400;"

First one to ensure everyone understands the execute capability, the second so you can send back meaningful codes (e.g. 401 for unauthorized).

@IvanGoncharov
Copy link
Member Author

@leebyron Can you please review this PR?

@IvanGoncharov IvanGoncharov merged commit 40a6a42 into graphql:master Apr 7, 2019
junminstorage pushed a commit to junminstorage/express-graphql that referenced this pull request Aug 14, 2020
This PR allows providing your own `execute` function instead of the default one from the `graphql-js`.
It’s a small change that adds a lot of flexibility since you can wrap standard `execute` and it opens new possibilities for middlewares. 

Personally, I need it to proxy calls to 3rd-party APIs.
It’s an alternative to graphql#253 but introduces less code and more flexible mechanism.
It’s very important for us as it will be the last change required to switch [graphql-faker](https://github.com/APIs-guru/graphql-faker) from custom forks of `graphql-js` and `graphql-express`.
@wincent @leebyron Can you please review it?
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants