Skip to content

fix: support graphql v17 (directives can be undefined) - #214

Open
jarkko wants to merge 1 commit into
ardeois:mainfrom
jarkko:fix/graphql-17-directives-optional
Open

fix: support graphql v17 (directives can be undefined)#214
jarkko wants to merge 1 commit into
ardeois:mainfrom
jarkko:fix/graphql-17-directives-optional

Conversation

@jarkko

@jarkko jarkko commented Jul 13, 2026

Copy link
Copy Markdown

Summary

Adds support for graphql v17 by fixing a crash that occurs during code generation, and widens the graphql peer dependency range to accept ^17.0.0.

The problem

In graphql-js 17, AST definition nodes no longer default their directives field to an empty array. When a type declares no directives, directives is now undefined instead of the [] that graphql-js 16 always produced.

The plugin detects the @oneOf directive by calling .some(...) directly on node.directives, which throws under graphql 17 when a type has no directives:

TypeError: Cannot read properties of undefined (reading 'some')

This crashes codegen at the InputObjectTypeDefinition visitor for any input type without directives (i.e. the common case), making the plugin unusable with graphql 17.

The fix

Use optional chaining at the two @oneOf detection sites in src/index.ts so a missing directives array is treated as "no directives" rather than crashing:

  • the InputObjectTypeDefinition visitor that collects @oneOf input types
  • the input-object mock function that checks hasOneOfDirective

Both changes are backward compatible: on graphql 14–16 directives is still [], so ?.some(...) behaves exactly as before.

Also widens the graphql peer dependency range to ^14.6.0 || ^15.0.0 || ^16.0.0 || ^17.0.0.

Testing

  • pnpm build passes
  • pnpm test passes (all existing tests/snapshots green)
  • eslint and tsc --noEmit clean

I kept this focused on the fix + peer range rather than adding a regression test: the existing suite builds schemas via graphql 16's buildSchema, which always populates directives: [], so the undefined case cannot be reproduced without bumping the project's graphql devDependency to 17 (which would churn the snapshot suite) or hand-constructing an AST. Happy to add a test in whatever form you'd prefer if you'd like one.

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