fix(api): type ResolverWrapper against graphql's native GraphQLResolveInfo - #561
Closed
smartive-nicolai[bot] wants to merge 1 commit into
Closed
Conversation
…eInfo
`@graphql-tools/utils` v12 augments `GraphQLResolveInfo` with two *required*
members, `getAbortSignal` and `getAsyncHelpers`. graphql 17 declares both
natively; graphql 16 declares neither.
Because `ResolverWrapper` was expressed through that package's
`IFieldResolver`, the utils v11 -> v12 bump made the type unimplementable on
graphql 16 for any consumer whose resolvers come from graphql-codegen (its
`ResolverFn` is typed with graphql's own resolve info):
error TS2322: Type 'GraphQLResolveInfo' is missing the following properties
from type 'GraphQLResolveInfo': getAbortSignal, getAsyncHelpers
graphql 16 is inside our declared peer range and the bump ships as a patch, so
this would have broken consumers silently. Express `ResolverWrapper` with
graphql's own `GraphQLResolveInfo` instead — which is what every other resolver
signature in the codebase already uses. `additionalResolvers` keeps using
`IResolvers`; that direction stays assignable and is unaffected.
The jest suite cannot catch this class of regression: ts-jest runs
transpile-only under `isolatedModules`, so no test file is ever type-checked,
and the only wrapper it exercises is an inferred `(resolver) => resolver` that
never pins down the parameter type. Add a type-level test checked by tsc
through a scoped `tsconfig.type-tests.json`, wired into `npm test` as
`test:types` so it runs on both graphql majors of the CI matrix.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stacked on #555 — merge this into that branch so the
@graphql-tools/utilsv12 bump and its fix land together.Replaces #560, which was the same branch targeted at
main; that one can be closed.The problem
@graphql-tools/utilsv12 augmentsGraphQLResolveInfowith two required members,getAbortSignalandgetAsyncHelpers. graphql 17 declares both natively; graphql 16 declares neither.ResolverWrapperwas expressed through that package'sIFieldResolver, so it inherited the augmented resolve info. graphql-codegen types itsResolverFnagainst graphql's ownGraphQLResolveInfo— so on graphql 16, every consumer that wraps codegen-shaped resolvers stops compiling:graphql 16 is inside our declared peer range (
^16.12.0 || ^17.0.0), and Renovate's commit is afix:so semantic-release ships it as a patch — this would have gone out silently in 29.4.1.Runtime is unaffected; this is type-level only.
The fix
Express
ResolverWrapperwith graphql's ownGraphQLResolveInfo, which is what every other resolver signature in the codebase (context.ts,resolvers/resolver.ts,resolvers/mutations.ts,api/execute.ts) already uses. The utils import was the odd one out.additionalResolverskeeps usingIResolvers— I checked that direction separately and it stays assignable on graphql 16, so it needs no change.Internals are unaffected:
getResolversreturnsRecord<string, any>and the wrapper is applied throughlodash.mapValues.Why CI didn't catch it
Two reasons, and the second is worth knowing independently:
(resolver) => resolver(tests/api/execute.spec.ts), which never pins down the parameter type.tsconfig.jest.jsonsetsisolatedModules: true, so no test file is ever type-checked. A type error anywhere undertests/is currently invisible.So this adds
tests/types/resolver-wrapper.ts, type-checked bytscthrough a scopedtsconfig.type-tests.jsonand wired intonpm testastest:types. It runs on both graphql majors of the existing matrix. Scoped deliberately rather than type-checking all oftests/:tsconfig.jest.json's**/*.tsinclude sweeps in the separatedocs/docusaurus project, which does not type-check standalone.knip.jsongains atests/types/**ignore, since the file is consumed bytscrather than imported.Verification
CI on this exact commit (
44c521b, run against basemainas #560) was fully green: all 21Test Packagematrix jobs plusLint project.Also ran locally against both graphql majors (
npm install graphql@^16.12.0/^17.0.0, as the matrix does):linttest:typestest:unit(157 tests)buildknipAnd confirmed the new test is load-bearing: with the
src/api/execute.tschange reverted,test:typesfails on graphql 16 with exactly the TS2322 above, while passing on graphql 17.Follow-up not included here
The
--legacy-peer-depscomment in.github/workflows/testing.ymlis stale: it blames@graphql-codegen/*for capping the graphql peer at^16, but those packages all allow^17now. The actual remaining holdout isgraphql-config(pulled in by@graphql-codegen/cli), whose latest release 5.1.6 still caps at^16— so the flag does have to stay, just for a different reason. I had this as a comment fix but the pushing GitHub App lacksworkflowspermission, so it needs a human commit.🤖 Generated with Claude Code