feat: pass abortSignal to resolvers #4261
Conversation
this allows e.g. passing the signal to fetch
✅ Deploy Preview for compassionate-pike-271cb3 ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
|
Hi @yaacovCR, I'm @github-actions bot happy to help you with this PR 👋 Supported commandsPlease post this commands in separate comments and only one per comment:
|
and add test
| @@ -990,6 +990,7 @@ export type GraphQLFieldResolver< | |||
| args: TArgs, | |||
| context: TContext, | |||
| info: GraphQLResolveInfo, | |||
| abortSignal: AbortSignal | undefined, | |||
There was a problem hiding this comment.
@yaacovCR I'm curious — how intentional is it that this was parameter defined as abortSignal: AbortSignal | undefined vs abortSignal?: AbortSignal? This means that any call in userland to a GraphQLFieldResolver (ie, that some user code may have plucked from a schema's field.resolve) will break if it doesn't explicitly pass in the signal.
In a sense I suppose that's good, because it means wrapping code (like this in Apollo Server) has to actually notice this new feature and not break it.
But I do wonder — why can't this just go on GraphQLResolveInfo rather than being a brand new field? To me, the arguments to a GraphQL-JS resolver have always felt like:
- the parent/source
- the arguments
- a grab-bag object for whatever metadata the app developer wants available to resolvers
- a grab-bag object for whatever metadata GraphQL JS wants available to resolvers
It feels like abortSignal fits perfectly fine on argument 4 without needing to have a brand new argument (which anything wrapping resolvers needs to pass through, etc). It also seems a bit more self-documenting to have the abortSignal come in an actual named field named abortSignal rather than just a 5th parameter.
(I think I suggested this back in 2022... I didn't realize the implementation didn't end up working that way.)
In graphql#4261 (not yet released in v17) we made abortSignal available to resolvers via a fifth argument to the field resolver. Among other things, this means that any code that processes schemas to wrap resolvers in other functions would have to be aware of this one new feature and specially thread through the new behavior. It also changed the TypeScript signature of GraphQLFieldResolver to *require* passing the fifth argument (even if undefined). But the field resolver interface already has a place for GraphQL-JS to put a grab-bag of helpful named objects for use by resolvers: `GraphQLResolveInfo`. This PR (which is not backwards compatible with v17.0.0-alpha.8, but is backwards-compatible with v16) moves the abortSignal into `GraphQLResolveInfo`.
this allows e.g. passing the signal to fetch
Note: the
abortSignalis now the fifth argument to a GraphQLFieldResolverFn. If no resolver if provided, and the parent is an object with a key for the field name with a value that is a function, theabortSignalwill be the fourth argument, as in the included test, with theparentaccessible via thethiskeyword.