fix(merge): preserve repeatable directives on schema definitions - #8349
Conversation
mergeGraphQLNodes collects directive definitions and passes them to every named-type merger, but not to mergeSchemaDefs — even though it already accepts a directives parameter (added in ardatan#7249, never wired up). Without the registry, mergeDirectives cannot tell that a schema-level directive is repeatable, so repeated instances on schema definitions and extensions are collapsed into one with merged arguments. Pass the registry through, enable the it.todo left in ardatan#7249, and add a regression test for repeatable directives on schema definitions across documents. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
🦋 Changeset detectedLatest commit: 7dabb1a The changes in this PR will be included in the next version bump. This PR includes changesets to release 6 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe schema merge path now passes collected directive definitions to ChangesSchema directive merging
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Thanks @aarne ! |
Description
mergeGraphQLNodescollects all directive definitions into a registry and passes it to every named-type merger, but not tomergeSchemaDefs— even though that function has accepted adirectivesparameter since #7249 (it was added to the signature but the call site was never updated). Without the registry,mergeDirectivescannot tell that a schema-level directive is declaredrepeatable, so repeated instances onschemadefinitions andextend schemaextensions are collapsed into a single directive with merged arguments.This PR passes the registry through at the call site (one line), enables the
it.todo('supports multiple schema extensions')left behind by #7249, and adds a regression test for a repeatable directive onschemadefinitions across documents (which fails without the fix).Federation
@linkmostly escaped this bug viarepeatableLinkImportsand the same-name/different-url carve-out inmergeDirectives; SDL-declaredrepeatable on SCHEMAdirectives had no such fallback.Fixes #8348
Type of change
How Has This Been Tested?
keeps repeatable directives on schema definitions across documents— fails without the one-line fix, passes with itsupports multiple schema extensionstodo testpackages/mergeandpackages/schemasuites pass (194 tests)Test Environment:
@graphql-tools/merge: master (9.2.2)Checklist:
🤖 Generated with Claude Code