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.

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

Question: do directives on objects get merged? #182

Closed
lastmjs opened this issue Oct 22, 2018 · 7 comments · Fixed by #196
Closed

Question: do directives on objects get merged? #182

lastmjs opened this issue Oct 22, 2018 · 7 comments · Fixed by #196
Labels
⏳waiting for release This issue or pull request is waiting to be released

Comments

@lastmjs
Copy link
Contributor

lastmjs commented Oct 22, 2018

I have a few schemas I'm merging together, and it doesn't seem to be merging in the following directive: directive @addId on OBJECT. Just wondering if it is known that object directives are not merged, or if I should do some more digging. Thanks!

@RichardLitt RichardLitt added the 🎙️question Further information is requested label Oct 24, 2018
@cfnelson
Copy link
Contributor

@lastmjs Directives were introduced in this PR #144 and there is a known problem with them -> #156 which I believe should be a simple fix.

Not sure if this relates to your issue sorry. Would be curious to hear about anything you find!

@Fi1osof
Copy link
Contributor

Fi1osof commented Nov 12, 2018

@lastmjs maybe fixed in #183

@cfnelson
Copy link
Contributor

@lastmjs I believe the latest fix by Fi1osof may do this for you, you should test out v1.5.8 . Let us know if it works for you!!! If not you could create a PR to fix/support this.

@kamilkisiela
Copy link
Collaborator

Should be fixed by #196

@kamilkisiela kamilkisiela mentioned this issue Jun 25, 2019
@kamilkisiela kamilkisiela added ⏳waiting for release This issue or pull request is waiting to be released and removed 🎙️question Further information is requested labels Jun 25, 2019
@kamilkisiela
Copy link
Collaborator

It's available now under v1.6.0-beta.2. Let us know if it works :)

@essaji
Copy link

essaji commented Jul 17, 2019

@kamilkisiela after upgrading to ^1.6.0-beta.2 now I am getting the following error when merging two schemas:

/node_modules/graphql-toolkit/dist/commonjs/epoxy/typedefs-mergers/directives.js:70
        throw new Error(`Unable to merge GraphQL directive "${node.name.value}". \nExisting directive:  \n\t${printedExistingNode} \nReceived directive: \n\t${printedNode}`);
        ^
Error: Unable to merge GraphQL directive "authorized". 
Existing directive:  
        directive @authorized(requires: [Roles] = [ADMIN]) on OBJECT | FIELD_DEFINITION 
Received directive: 
        directive @authorized(requires: String) on OBJECT | FIELD_DEFINITION

@kamilkisiela
Copy link
Collaborator

kamilkisiela commented Jul 18, 2019

Yes because you can't merge them because they both have different arguments. Previously (before beta), merge-graphql-schema kept those two definitions which was invalid, there should be one definition per each entity.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
⏳waiting for release This issue or pull request is waiting to be released
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants