-
Notifications
You must be signed in to change notification settings - Fork 1k
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
Replace merge-graphql-schemas with graphql-tools #1230
Comments
We've got tests for this that'll help with the conversion:
|
Note that by implementing this replacement, if directives work, then could use instead of graphql-shield to protect queries and mutations as described in #965 and explained in https://www.apollographql.com/docs/apollo-server/schema/creating-directives/#enforcing-access-permissions
and also in https://www.graphql-tools.com/docs/schema-directives/#enforcing-access-permissions
|
Hey @peterp, I would like to work on this issue. Is this issue up for taking? |
Hi @himankpathak! Would be great to have your help! Peter is on vacation for the next week. Since he's the lead on this part of the code, we won't be able to have him Review and Merge until the week after he's back (still in time for Hacktoberfest). But if you want to get started and open a PR, both I and @dthyresson could likely guide you on your way. Does that sound good to you? Let me know and I'll assign this to you. Lastly, if you haven't seen it yet here's some helping getting started information: #1266 |
Thanks @thedavidprice, |
@himankpathak yes, indeed! Would you like me to assign this to you? |
Sure @thedavidprice , you can assign me the issue. |
We're using
merge-graphql-schemas
which is now deprecated:redwood/packages/api/src/makeMergedSchema/makeMergedSchema.ts
Line 7 in e5574f9
Let's rather use graphql-tools/merge package:
https://www.graphql-tools.com/docs/migration-from-merge-graphql-schemas/
I think this may be required for directives to work properly.
The text was updated successfully, but these errors were encountered: