Skip to content

Add support for directives on directive definitions#4521

Open
BoD wants to merge 18 commits intographql:16.x.xfrom
BoD:directive-on-directive-definitions
Open

Add support for directives on directive definitions#4521
BoD wants to merge 18 commits intographql:16.x.xfrom
BoD:directive-on-directive-definitions

Conversation

@BoD
Copy link
Copy Markdown

@BoD BoD commented Jan 6, 2026

Allow directives on directive definitions, based on this spec PR which introduces this syntax:

directive @onDirective on DIRECTIVE_DEFINITION

directive @foo @onDirective on OBJECT

directive @baz @deprecated(reason: "...") on OBJECT

extend directive @quux @deprecated(reason: "...")

Disclaimer

First time on this codebase and am also not a JS/TS person, so obvious mistakes and/or missing pieces are very likely 😅 Any help and feedback to improve this PR are very welcome 🙏. The tests seem to pass but additional tests may be needed.

@BoD BoD requested a review from a team as a code owner January 6, 2026 17:46
@vercel
Copy link
Copy Markdown

vercel Bot commented Jan 6, 2026

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

A member of the Team first needs to authorize it.

Comment thread src/language/parser.ts Outdated
Comment thread src/language/parser.ts Outdated
@leebyron
Copy link
Copy Markdown
Contributor

leebyron commented Apr 2, 2026

Update includeDeprecated arg per @benjie 's comments, then we'll merge

@BoD
Copy link
Copy Markdown
Author

BoD commented Apr 3, 2026

Just made the change for includeDeprecated: Boolean! + added one test.

Copy link
Copy Markdown

@jerelmiller jerelmiller left a comment

Choose a reason for hiding this comment

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

Had a few suggestions, mostly around types, but otherwise looks good from my end!

You probably want to get a proper review from someone who knows the conventions in this codebase better than I do, but wanted to try and help move this along in some way!

Comment thread src/language/parser.ts Outdated
Comment thread src/language/parser.ts Outdated
Comment thread src/utilities/__tests__/extendSchema-test.ts Outdated
Comment thread src/type/directives.ts
Comment thread src/utilities/extendSchema.ts Outdated
Comment thread src/utilities/extendSchema.ts Outdated
Comment thread src/utilities/extendSchema.ts Outdated
Comment thread src/utilities/extendSchema.ts Outdated
Comment thread src/utilities/extendSchema.ts Outdated
BoD and others added 6 commits April 7, 2026 10:44
Co-authored-by: Jerel Miller <jerelmiller@gmail.com>
Co-authored-by: Jerel Miller <jerelmiller@gmail.com>
Co-authored-by: Jerel Miller <jerelmiller@gmail.com>
Co-authored-by: Jerel Miller <jerelmiller@gmail.com>
Co-authored-by: Jerel Miller <jerelmiller@gmail.com>
@benjie
Copy link
Copy Markdown
Member

benjie commented Apr 7, 2026

I've not fully reviewed this PR, but the changes since Lee's review all look fine to me - thanks @BoD! 👍

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.

5 participants