Skip to content

fix(api): type ResolverWrapper against graphql's native GraphQLResolveInfo - #561

Closed
smartive-nicolai[bot] wants to merge 1 commit into
renovate/major-graphql-tools-monorepofrom
fix/resolver-wrapper-native-resolve-info
Closed

fix(api): type ResolverWrapper against graphql's native GraphQLResolveInfo#561
smartive-nicolai[bot] wants to merge 1 commit into
renovate/major-graphql-tools-monorepofrom
fix/resolver-wrapper-native-resolve-info

Conversation

@smartive-nicolai

Copy link
Copy Markdown
Contributor

Stacked on #555 — merge this into that branch so the @graphql-tools/utils v12 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/utils v12 augments GraphQLResolveInfo with two required members, getAbortSignal and getAsyncHelpers. graphql 17 declares both natively; graphql 16 declares neither.

ResolverWrapper was expressed through that package's IFieldResolver, so it inherited the augmented resolve info. graphql-codegen types its ResolverFn against graphql's own GraphQLResolveInfo — so on graphql 16, every consumer that wraps codegen-shaped resolvers stops compiling:

error TS2322: Type '(resolver: ResolverFn<...>) => ResolverFn<...>' is not assignable to type 'ResolverWrapper'.
  Types of parameters 'resolver' and 'resolver' are incompatible.
    Types of parameters 'info' and 'info' are incompatible.
      Type 'GraphQLResolveInfo' is missing the following properties from type 'GraphQLResolveInfo': getAbortSignal, getAsyncHelpers

graphql 16 is inside our declared peer range (^16.12.0 || ^17.0.0), and Renovate's commit is a fix: 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 ResolverWrapper with graphql's own GraphQLResolveInfo, 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.

additionalResolvers keeps using IResolvers — I checked that direction separately and it stays assignable on graphql 16, so it needs no change.

Internals are unaffected: getResolvers returns Record<string, any> and the wrapper is applied through lodash.mapValues.

Why CI didn't catch it

Two reasons, and the second is worth knowing independently:

  1. The only wrapper the suite exercises is an inferred (resolver) => resolver (tests/api/execute.spec.ts), which never pins down the parameter type.
  2. ts-jest runs transpile-onlytsconfig.jest.json sets isolatedModules: true, so no test file is ever type-checked. A type error anywhere under tests/ is currently invisible.

So this adds tests/types/resolver-wrapper.ts, type-checked by tsc through a scoped tsconfig.type-tests.json and wired into npm test as test:types. It runs on both graphql majors of the existing matrix. Scoped deliberately rather than type-checking all of tests/: tsconfig.jest.json's **/*.ts include sweeps in the separate docs/ docusaurus project, which does not type-check standalone.

knip.json gains a tests/types/** ignore, since the file is consumed by tsc rather than imported.

Verification

CI on this exact commit (44c521b, run against base main as #560) was fully green: all 21 Test Package matrix jobs plus Lint project.

Also ran locally against both graphql majors (npm install graphql@^16.12.0 / ^17.0.0, as the matrix does):

graphql 16.14.2 graphql 17.0.2
lint pass pass
test:types pass pass
test:unit (157 tests) pass pass
build pass pass
knip pass pass

And confirmed the new test is load-bearing: with the src/api/execute.ts change reverted, test:types fails on graphql 16 with exactly the TS2322 above, while passing on graphql 17.

Follow-up not included here

The --legacy-peer-deps comment in .github/workflows/testing.yml is stale: it blames @graphql-codegen/* for capping the graphql peer at ^16, but those packages all allow ^17 now. The actual remaining holdout is graphql-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 lacks workflows permission, so it needs a human commit.

🤖 Generated with Claude Code

…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>
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.

1 participant