Skip to content

feat: add validateFullSchema and assertValidFullSchema#4798

Open
vishwakt wants to merge 3 commits into
graphql:17.x.xfrom
vishwakt:add-validate-full-schema
Open

feat: add validateFullSchema and assertValidFullSchema#4798
vishwakt wants to merge 3 commits into
graphql:17.x.xfrom
vishwakt:add-validate-full-schema

Conversation

@vishwakt

@vishwakt vishwakt commented Jun 4, 2026

Copy link
Copy Markdown

Motivation

Closes #4415.

validateSchema() validates a source schema and rejects any name beginning with __, since those are reserved by GraphQL introspection. However, tools that work with a full schema (for example one reconstructed from an introspection result, as in GraphiQL or codegen) may legitimately encounter reserved names, including ones added by a newer version of GraphQL than the running version of graphql-js implements.

The reproducer from the issue fails today purely on the reserved-name rule, even though the SDL is otherwise valid:

const schema = buildSchema(`
  type Query { hello: String }

  enum __ErrorBehavior { NULL PROPAGATE HALT }

  directive @behavior(onError: __ErrorBehavior) on SCHEMA
`);

validateSchema(schema);
// GraphQLError: Name "__ErrorBehavior" must not begin with "__", which is reserved by GraphQL introspection.

See the Full Schemas RFC for the broader context on source vs. full schemas.

Changes

  • Add experimentalValidateFullSchema(schema) and experimentalAssertValidFullSchema(schema), mirroring validateSchema / assertValidSchema.
  • They run every existing type-system check except the reserved-name (__) restriction, so a full schema validates without spurious errors.
  • Internally, both modes share the existing validation pipeline via a single allowReservedNames flag threaded through SchemaValidationContext.
  • The two modes use independent validation caches (__validationErrors vs. __fullSchemaValidationErrors) so their results never collide regardless of call order.
  • The functions use the experimental name prefix (matching experimentalExecuteIncrementally) to signal the API may change while behavior and test coverage for full schemas converge.

Tests

Added coverage for: the issue's reproducer, reserved names on types/fields/args/enum values/input fields, that non-name errors are still reported when validating a full schema, independent caching, and the assert variant.

npm run check, npm run lint, and the full test suite pass locally.

validateSchema validates a source schema and rejects any name beginning
with "__", which is reserved by GraphQL introspection. Tools that work
with a full schema (for example one reconstructed from introspection)
may legitimately encounter reserved names, including ones added by a
newer version of GraphQL than this library implements.

Add validateFullSchema and assertValidFullSchema, which run the same
checks as validateSchema except that reserved names are permitted. The
two modes use separate validation caches so their results never collide.

Closes graphql#4415
@vercel

vercel Bot commented Jun 4, 2026

Copy link
Copy Markdown

@vishwakt is attempting to deploy a commit to the The GraphQL Foundation Team on Vercel.

A member of the Team first needs to authorize it.

@linux-foundation-easycla

linux-foundation-easycla Bot commented Jun 4, 2026

Copy link
Copy Markdown

CLA Signed
The committers listed above are authorized under a signed CLA.

  • ✅ login: vishwakt / name: Vishwak Thatikonda (44c9266)

@martinbonnin martinbonnin left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM although not a graphql-js expert.

Might be worth marking this function as experimental until we collectivelly converge on a bigger test suite?

Per review feedback, rename validateFullSchema and assertValidFullSchema
to experimentalValidateFullSchema and experimentalAssertValidFullSchema,
matching the existing experimental* naming convention used for
experimentalExecuteIncrementally. This signals the API may change while
the behavior and test coverage for full schemas converge.
@vishwakt

vishwakt commented Jun 4, 2026

Copy link
Copy Markdown
Author

Good idea, thanks. I marked both functions as experimental by adopting the existing experimental name prefix convention (as used by experimentalExecuteIncrementally / experimentalExecuteRootSelectionSet), since that seems to be the established pattern in the repo for signalling an unstable API rather than a JSDoc tag. They're now experimentalValidateFullSchema and experimentalAssertValidFullSchema, and I added a note in the JSDoc too. Happy to revisit the names once the behavior and test coverage for full schemas converge.

@martinbonnin martinbonnin left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thanks for addressing the feedback.

Pinging @yaacovCR @benjie @jerelmiller for more expert eyes.

@benjie benjie left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure this is the right solution. For example, the schema:

scalar __S
type __Q {
  __i: __S
}
schema {
  query: __Q
}

should be invalid.

Similarly, the less ridiculous schema:

type Query {
  int: __Type
}

should also be invalid.

I understand what we're trying to achieve here, but I don't think simply turning off the "no double underscore" validation rule is sufficient, there's a lot of assumptions baked into that rule that if we turn them off we'll need to re-apply other ways (e.g. "starting at the root operation types and without traversing or including metafields, no __-prefixed field, argument or type should be reachable").

I think we need to invest a lot of thought into what a "full schema" means for GraphQL.js

A simpler solution might be for GraphiQL to accept an option that tells it to assume the schema is valid.

Address review feedback: rather than disabling the reserved-name rule
outright, a full schema now rejects a reserved (__-prefixed) name only
when it is reachable from the root operation types through ordinary
fields, arguments, and their types.

The introspection types and other implementation-provided definitions
are only reachable through the __schema, __type, and __typename
meta-fields, which are never part of a type's fields, so they are not
traversed and remain valid. A user type or field exposing an
introspection type, or a reserved root operation type, is still
rejected.
@vishwakt

vishwakt commented Jul 9, 2026

Copy link
Copy Markdown
Author

Thanks @benjie, that's a good catch and I agree the blanket approach was wrong. I've reworked it to implement the rule you described.

Instead of disabling the reserved-name check, a full schema now reports a reserved (__-prefixed) name only when it is reachable from the root operation types through ordinary fields, arguments, and their types. Because the introspection types (and other implementation-provided definitions) are only reachable via the __schema / __type / __typename meta-fields, which are never part of a type's getFields(), they are never traversed and remain valid.

Both of your examples are now rejected:

scalar __S
type __Q { __i: __S }
schema { query: __Q }
type Query { int: __Type }

while a reserved definition that is not part of the queryable surface (e.g. the __ErrorBehavior enum used only by a directive argument in the original issue, or an unreachable reserved type) is still accepted.

Added tests for both of your examples plus reachable reserved field/argument names, unreachable reserved definitions, and independent caching. This still doesn't settle the larger "what is a full schema" design question, but it should at least make the validation sound rather than simply permissive. Happy to keep iterating.

@benjie

benjie commented Jul 10, 2026

Copy link
Copy Markdown
Member

In case you didn't already, you should also target all non-type-system directives (e.g. directive @behavior(onError: __ErrorBehavior) on SCHEMA should be ignored, because SCHEMA is type system, but directive @flibble(__foo: __Bar) on FIELD should be rejected).

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.

Allow to validate a full schema

3 participants