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

Thoughts on merging schemas that have their own Relay node lookups #1013

Closed
fgrehm opened this issue Nov 29, 2018 · 5 comments
Closed

Thoughts on merging schemas that have their own Relay node lookups #1013

fgrehm opened this issue Nov 29, 2018 · 5 comments

Comments

@fgrehm
Copy link

fgrehm commented Nov 29, 2018

Hey there!

I'm currently evaluating schema stitching for some of our company needs and one of the things that came up was the fact that each of our schemas will have their own Query.node(s) fields and when merging the schemas together there's no easy way for us to delegate to the appropriate schema based on the ID provided.

I've spent a lot of time searching around and haven't found anything specific to the problems we're trying to solve so I came up with a solution that seems to work for our use cases. The result of that spike is available at fgrehm/spike-gql-stitch-relay-nodes and you can play with it here. PS: I come from a Ruby background and I'm kinda new to "GraphQL world" in nodejs so excuse my JS :)

At a high level, this is what I've done:

  1. Applied some transformation to individual schemas:
    • Removed Query.nodes
    • Renamed Query.node to Query.[schemaPrefix]Node
    • Prefixed types with SCHEMA_
  2. Merged schemas with a custom extension that defines a "join" and the new "generic" node(s) fields:
  3. Removed Query.[schemaPrefix]Node fields from stitched schema since that's an "internal implementation detail" and we don't want clients to make use of that.
  4. Lastly, we used the graphql-add-middleware package to change all of the implementations of the Node interface to prefix ids with an urn. That urn is what we use to determine which schema the app needs to delegate to without giving the "gateway API" specific knowledge about where to find each individual type.

I know that's a lot of information to digest but I really couldn't find any material around this type of thing so any advice or recommendation on how to best approach this problem would be great. I'd also be happy to contribute some official documentation this problem to save other peoples times :D

@kommander
Copy link

Interesting approach, I have been looking for a solution to this as well. Here are my thoughts:

  • Instead of prefixing types with the schema name, why not use namespaces? Wrap them in a super-type maybe to not have to anticipate the prefix and think about it when writing your schema.
  • const { includes, forEach, map } = require("lodash"); Why not use native Array methods?
  • Just a heads up: delegateToSchema and mergeSchemas turns out to be really expensive, and a Promise.all with an arbitrary Number of ids triggering further delegations could easily kill your server.
  • That said, when stitching the schemas, you have access to each of the node resolvers, which you could call directly instead of delegating to save some overhead.

@yaacovCR
Copy link
Collaborator

Agree with @kommander that for your use case, schema stitching may not be the right tool, as you may be able to simple merge all of your local schemas.

If you cannot merge the local schemas, and must rely on proxying, I would suggest to avoid delegating to the merged schema, because that introduces an extra round of delegation. Rather, you may be able to solve your context problem with a few context conversion functions.

Namespacing with a super-type is supported in graphql-tools@next via WrapType, but it currently does not work as expected with queries or subscriptions. One may have to wait for the spec to take up true namespacing, graphql/graphql-spec#163.

Folding this issue into the docs issue #1316.

@yaacovCR yaacovCR mentioned this issue Mar 30, 2020
10 tasks
@yaacovCR
Copy link
Collaborator

Reopening this issue to track progress on an example demonstrating existing functionality and documenting any gaps.

@yaacovCR yaacovCR reopened this May 13, 2020
@sibelius
Copy link

I have a similar use case (related to #1663)

but instead of many graphql server, I'd like to share types among graphqls, and avoid having all types exposed all types, only the ones required by the graphql itself

@yaacovCR
Copy link
Collaborator

yaacovCR commented Oct 6, 2020

Closing this stale issue, feel free to reopen if there's anything we can be of further help with...

@yaacovCR yaacovCR closed this as completed Oct 6, 2020
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

No branches or pull requests

4 participants