Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

duplicate directives when merging schemas since version 5.0.0 #1656

Closed
tapaderster opened this issue Jun 17, 2020 · 5 comments · Fixed by #1665
Closed

duplicate directives when merging schemas since version 5.0.0 #1656

tapaderster opened this issue Jun 17, 2020 · 5 comments · Fixed by #1665

Comments

@tapaderster
Copy link

When merging schemas with common directives, in the stitched schema we are getting duplicate directives.

Is there a way to have just unique list of directives in the stitched graphql schema?
@yaacovCR
Copy link
Collaborator

Are you using mergeDirectives: true?

Best would be a minimal reproduction with the result/error you are getting and what you would like to get?

We may need a onDirectiveConflict option like onTypeConflict, but I want to make sure that I understand exactly the bug fix/feature you are requesting.

@tapaderster
Copy link
Author

yeah. I am using mergeDirectives: true.

I am trying to merge fully functional sdls, each containing it's own type directive definitions.
example: include, skip, deprecated, etc.

I guess the prev versions of graphql-tools had a bug with mergeDirectives since stitched schema only had these 3 directives (excluded all custom directives we had).

But now since version 5.0.0, I am getting all the directives merged (which it should be). But it means in the generated schema I am getting duplicate include, skip, deprecated directives.

I will try to add a minimal repro schema.

yaacovCR added a commit that referenced this issue Jun 21, 2020
@yaacovCR
fix(stitching): avoid duplicate directives
a279c84 
Fixes #1656.

Later directives with the same name should override earlier directives rather than causing an error.

Directives with the same names from a later subschema will override an earlier subschema. Directives from typedefs will override both.

Mixing of legacy schemas argument and newer subschemas, typeDefs, and types arguments should respect this same order.

We can consider adding onDirectiveConflict option to match onTypeConflict for customization.
@yaacovCR yaacovCR mentioned this issue Jun 21, 2020
Merged
yaacovCR added a commit that referenced this issue Jun 21, 2020
@yaacovCR
fix(stitching): avoid duplicate directives (#1665)
bff7610 
Fixes #1656.

Later directives with the same name should override earlier directives rather than causing an error.

Directives with the same names from a later subschema will override an earlier subschema. Directives from typedefs will override both.

Mixing of legacy schemas argument and newer subschemas, typeDefs, and types arguments should respect this same order.

We can consider adding onDirectiveConflict option to match onTypeConflict for customization.
@yaacovCR yaacovCR reopened this Jun 21, 2020
@yaacovCR yaacovCR added the waiting-for-release Fixed/resolved, and waiting for the next stable release label Jun 21, 2020
@yaacovCR
Copy link
Collaborator

Thanks again for reporting this!

#1665 attempts a simple fix to avoid duplicates without yet introducing an onDirectiveConflict option.

This is available as alpha in npm: 6.0.11-alpha-ab03f0e.0

Quickly update your package.json by running:

npx match-version @graphql-tools 6.0.11-alpha-ab03f0e.0

@tapaderster
Copy link
Author

Appreciate the quick turnaround on this.

@ardatan
Copy link
Owner

ardatan commented Jun 25, 2020

Available in v6.0.11!

@ardatan ardatan closed this as completed Jun 25, 2020
@yaacovCR yaacovCR removed the waiting-for-release Fixed/resolved, and waiting for the next stable release label Oct 25, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix(stitching): avoid duplicate directives ardatan/graphql-tools
3 participants
@tapaderster @yaacovCR @ardatan