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

Why does mergeSchemas use delegatingResolvers? #1035

Closed
kommander opened this issue Dec 26, 2018 · 8 comments
Closed

Why does mergeSchemas use delegatingResolvers? #1035

kommander opened this issue Dec 26, 2018 · 8 comments

Comments

@kommander
Copy link

Happy holidays,

I was wondering why mergeSchemas uses a delegating resolver here https://github.com/apollographql/graphql-tools/blob/master/src/stitching/mergeSchemas.ts#L446. Why is this necessary? Simple cases like this one https://github.com/kommander/graphql-tool-repro won't work this way, which is frustrating.

Anyway, have a good time y'all.

@kommander kommander changed the title Why does mergeSchemas use delegatingResolvers? 🎅🎁 Why does mergeSchemas use delegatingResolvers? Feb 5, 2019
@kommander
Copy link
Author

kommander commented Feb 5, 2019

I hit another Issue with mergeSchemas. A super simple use case which you can test here https://github.com/kommander/graphql-tool-repro/tree/extended-fields-not-using-base-resolver.

The merging behaves differently for typeDefs and schemas created with makeExecutableSchema where my guess is the delegating resolvers are causing this weird behaviour again. Any pointers would be appreciated.

EDIT: Created separate issue #1060

@dncrews
Copy link

dncrews commented Feb 6, 2019

From what I can tell the assumptions you are making are incorrect: while communicating between schemas, you ONLY have the GraphQL schema of the other party to work with. You do NOT have access to the raw data that schema uses. In your extendResolvers, you are returning the raw objects in the same way that the remote server would have, so you're kind of cheating there. If you assume that these schemas do NOT live on the same server, and do NOT share datasources, the correct stitching looks like this:

kommander/graphql-tool-repro#2

dncrews added a commit to dncrews/graphql-tool-repro that referenced this issue Feb 6, 2019
ardatan/graphql-tools#1035 (comment)

(sorry for the formatting changes. That's just my editor)
@kommander
Copy link
Author

Thanks for your input, very much appreciated.

Having a remote schema, I get your point, you only have the result you get from the remote. When stitching schemas locally though, I would know I am stitching local schemas, so delegation would not be necessary and could extend the actual type in the local schema?

Use case is: having multiple schemas locally, transforming them before merge, therefor they have to be created with makeExecutableSchema first, then merging them to have one local schema. When only using local schemas, the delegation becomes unnecessary overhead. Or are there advantages of delegation to local schemas I don't see here?

@dncrews
Copy link

dncrews commented Feb 7, 2019

I've been doing some stitched local schemas for larger schemas that need to be broken up in the future (logical separation). We're doing microservices, but the legacy stuff is still too big. It's hard to not fall into this pattern of sharing datasources, but it makes for a much more modular codebase. I don't have the numbers yet to identify what the performance hit is, but I'm working on that.

@kommander
Copy link
Author

I will try to get some performance monitoring up as well, we just started using Apollo-Engine, the data we get out of it is quite usable, maybe I can get some A/B testing for the modules we have split up logically. Would be interesting. We have quite a set of services as well, which are consumed by different resolvers, so all the heavy lifting is done by the REST services and the GQL layer stitches them together. When using logically separated modules, which could also profit from using the same Dataloader, it makes sense to have them share data locally, instead of delegating calls to other schemas just because they could be remote.

@dncrews
Copy link

dncrews commented Feb 7, 2019

Definitely makes it easier. You just have to be careful in situations like this one where it's easy to forget that through the stitching, your contract is GraphQL, and not the raw data.

@kommander
Copy link
Author

kommander commented Feb 7, 2019

You just have to be careful in situations like this one where it's easy to forget that through the stitching, your contract is GraphQL, and not the raw data.

Important insight, will keep that in mind.

@yaacovCR
Copy link
Collaborator

Schema stitching is a proxying solution meant for cases in which you don't have access to remote schema or would like to transform it. It is not primarily intended for local schema delegation, although that always works, because remote schemas are made to appear local.

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

3 participants