Skip to content

parser, validator: accept directives on directive definitions - #515

Merged
StevenACoffman merged 2 commits into
masterfrom
directive-definition-directives-correction
Oct 21, 2026
Merged

StevenACoffman merged 2 commits into
masterfrom
directive-definition-directives-correction

Conversation

@StevenACoffman

Copy link
Copy Markdown
Collaborator

Supersedes #510 (by @stealth-rklopper), which I could not update. Fixes #506.

This carries the original commit from #510 unchanged (authorship preserved), rebased onto
current master, plus one follow-up commit.

What #510 does

The working draft of the GraphQL spec lets a directive definition take directives, and lets a
schema extend a directive with more of them (graphql/graphql-spec#1206):

directive @old(reason: String) @deprecated(reason: "use @new") on FIELD_DEFINITION

extend directive @old @tag(name: "b")
  • The parser reads the directives of a directive definition into
    DirectiveDefinition.Directives, and the location DIRECTIVE_DEFINITION.
  • The parser reads a directive extension into SchemaDocument.DirectiveExtensions.
  • ValidateSchemaDocument merges each extension's directives into its directive, and refuses an
    extension of an undefined directive.
  • The validator checks a directive's directives at DIRECTIVE_DEFINITION. A non-repeatable
    directive may appear once across a directive and its extensions, as graphql-js 17's
    UniqueDirectivesPerLocationRule checks.
  • The formatter writes both forms.

The grammar matches graphql-js 17 (parseDirectiveDefinition / parseDirectiveExtension).

Follow-up commit

Limit the prelude change to the DIRECTIVE_DEFINITION location. #510 also added
graphql-js 17's directive-deprecation introspection to the prelude (__Directive.isDeprecated,
__Directive.deprecationReason, __Schema.directives(includeDeprecated:)). Those break gqlgen:
with gqlgen v0.17.95, a minimal project fails to build because introspection.Directive has no
such fields and Schema.Directives() takes no argument:

graph/generated.go:604:38: cannot call pointer method IsDeprecated on __DirectiveResolver
graph/generated.go:627:38: cannot call pointer method DeprecationReason on __DirectiveResolver
graph/prelude.resolvers.go:28:6: __DirectiveResolver redeclared in this block

With this PR the same project generates and builds. The introspection fields should land
together with gqlgen support, in a separate change.

The prelude is generated by validator/imported/export.js from graphql-js v16.13.2, which has
neither the location nor the @deprecated location. export.js now applies both through
applyPreludeDeviations, so regenerating the prelude keeps them, and fails loudly if a
graphql-js upgrade changes the text they patch.

Also:

  • Update the singleLocation doc on validateDirectives, which said merged
    definition-plus-extension lists skip the non-repeatable check; directive definitions are now
    the exception.
  • Add tests for arguments on a directive extension (rejected), extending a builtin directive,
    and formatting a comment before an extension.

The builtin baselines of the formatter change with the prelude.

stealth-rklopper and others added 2 commits October 20, 2026 21:57
- Parse the directives of a directive definition after its arguments,
  and the location DIRECTIVE_DEFINITION.
- Parse a directive extension into SchemaDocument.DirectiveExtensions.
- Merge the directives of each extension into its directive, and
  refuse an extension of a directive that is not defined.
- Validate the directives of a directive at DIRECTIVE_DEFINITION.
- Allow @deprecated on a directive definition, and add the
  introspection fields of deprecated directives to the prelude.
- Write both forms in the formatter.
The previous commit hand-edited the generated prelude to add directive
deprecation to introspection (__Directive.isDeprecated,
__Directive.deprecationReason and __Schema.directives(includeDeprecated:)).
The pinned graphql-js has none of these, so the next export would drop
them. They would also break gqlgen, whose introspection.Directive and
Schema.Directives() have no matching fields or arguments. Drop them; they
belong in a separate change coordinated with gqlgen.

Keep the DIRECTIVE_DEFINITION location and the @deprecated location that
directives on directive definitions need, and have export.js apply them
through applyPreludeDeviations so regenerating the prelude preserves them.

Also update the singleLocation doc on validateDirectives, which said merged
definition-plus-extension lists skip the non-repeatable check; directive
definitions are now the exception. Add tests for arguments on a directive
extension, extending a builtin directive, and formatting a comment before
an extension.
@StevenACoffman
StevenACoffman merged commit cb6ec58 into master Oct 21, 2026
7 of 8 checks passed
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.

Support directives on directive definitions and extend directive from the working draft

2 participants