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

Improve doc: graphl-js does not support interfaces and unions with buildSchema #1379

Open
Franco992 opened this Issue Jun 9, 2018 · 5 comments

Comments

Projects
None yet
2 participants
@Franco992
Copy link

Franco992 commented Jun 9, 2018

As far as I understand, it is the recommended approach to use a root object when we build the schema with buildSchema, because we cannot associate resolver functions when using buildSchema. With this approach the default resolvers are used (AFAIK). The problem is, that we can not use interfaces or union types (defined with SDL), because we can not associate the resolveType function - is that correct? If yes, how can we provide resolveType when using graphql-js with SDL?

The problem here is, that on graphql.org the examples are written for apollo-server, but the graphl-js examples show another approach!

Is there a reason why graphql-js does not use the same approach as graphql-tools (makeExecuteableSchema)? So writing "explicit" resolver functions and not using default resolvers?
What is the recommended approach (or the spec compliant approach)? It seems, that default resolvers are not specified in the spec?

This seems to be very confusing for people, when reading graphql.org and then implementing a graphl-js server.

I appreciate every answer, that clarifies some of my points! Thank you very much!

@Franco992 Franco992 changed the title Improve doc: graphl-js does not support interfaces and enums with buildSchema Improve doc: graphl-js does not support interfaces and unions with buildSchema Jun 9, 2018

@IvanGoncharov

This comment has been minimized.

Copy link
Collaborator

IvanGoncharov commented Jun 11, 2018

As far as I understand, it is the recommended approach to use a root object when we build the schema with buildSchema, because we cannot associate resolver functions when using buildSchema. With this approach, the default resolvers are used (AFAIK).

@Franco992 Right 👍

The problem is, that we cannot use interfaces or union types (defined with SDL), because we can not associate the resolveType function - is that correct? If yes, how can we provide resolveType when using graphql-js with SDL?

Default resolveType is looking for __typename inside object so you either assign such property to rootObj or use getter function if you are using ES6 classes.
You actually motivated me to rewrite SWAPI example from this repo see #1384
https://github.com/APIs-guru/graphql-js/blob/432adaea861d649c47c89b95ad2d0ffd4ab50e37/src/__tests__/starWarsSchema.js#L142-L144

Is there a reason why graphql-js does not use the same approach as graphql-tools (makeExecuteableSchema)? So writing "explicit" resolver functions and not using default resolvers?
What is the recommended approach (or the spec compliant approach)? It seems that default resolvers are not specified in the spec?

Default resolver approach used even in graphql-tools otherwise you would need to write resolver for every field. My understanding is that makeExecuteableSchema was written after buildSchema and I didn't find any attempt to contribute such functionality to graphql-js.

What is the recommended approach (or the spec compliant approach)? It seems that default resolvers are not specified in the spec?

Specification leaves such technical details to implementers so they are free to implement in a way that appropriates to their language/platform:
http://facebook.github.io/graphql/June2018/#sec-Value-Resolution

So having default resolvers is completely valid according to the spec. Moreover graphql-js is the reference implementation of GraphQL that mean 3rd-party implementations of GraphQL encouraged to also implement default resolvers in a similar way.

Personally, I like to organize my resolvers as ES6 classes in the absolute majority of cases. I also found that it's easier to explain how GraphQL is working using ES6 classes than using global resolvers separated from rootValue.

At the same time, I think we need to support adding properties that you can't supply in SDL during buildSchema/extendSchema and this would include resolve function. See this comment: #1384 (comment)

@leebyron Would be great to hear your thoughts on this topic.

@Franco992

This comment has been minimized.

Copy link
Author

Franco992 commented Jun 11, 2018

@IvanGoncharov Thank you very much for you detailed answer!

Default resolveType is looking for __typename inside object so you either assign such property to rootObj or use getter function if you are using ES6 classes.
You actually motivated me to rewrite SWAPI example from this repo see #1384

Ah ok, that makes it "useable" again. Thanks for the clarification

Default resolver approach used even in graphql-tools otherwise you would need to write resolver for every field. My understanding is that makeExecuteableSchema was written after buildSchema and I didn't find any attempt to contribute such functionality to graphql-js.

I formulated that a little bit complicated. Sorry for that. What I meant was, that there is no way to provide explicit resolvers, when using graphql-js (using the SDL approach).

AFAIK not graphl-tools has some kind of default resolver approach, it "just" attaches the given resolvers to the schema instance. Because e.g. apollo uses graphql-js under the hood, the same default resolver strategy is used. But my point here is, that the spec says nothing about default resolvers.

Specification leaves such technical details to implementers so they are free to implement in a way that appropriates to their language/platform:
http://facebook.github.io/graphql/June2018/#sec-Value-Resolution

The spec states the function ResolveFieldValue(objectType, objectValue, fieldName, argumentValues), where the second argument is the objectValue. So doesn't that mean, that every implementation has to provide the possibilities to provide (by the dev) such a resolver function (with such a signature) for every field?

Is it safe to say (in general for all graphql spec compliant implementations, independent of programming language, ...), that there has to be a resolver function for every field?

I know that some points seem to be nitpicking. I just want to understand that in detail, not criticize.

Thanks for your help.

@IvanGoncharov

This comment has been minimized.

Copy link
Collaborator

IvanGoncharov commented Jun 11, 2018

@Franco992 Before we go deep into GraphQL specification. Also lets state a few points:

  • You can register resolver if you create your schema using GraphQL*Type.
  • You can't do the same with schema defined in SDL without hacks
  • Until yesterday SDL was an experimental feature and not included in the specification: https://github.com/facebook/graphql/releases/tag/June2018
  • Personaly I think it should be possible to attach resolvers to SDL schema but I don't think we should replicate makeExecuteableSchema design.

I think that this functionality is missing because no one design, implement and submit a PR.

@IvanGoncharov

This comment has been minimized.

Copy link
Collaborator

IvanGoncharov commented Jun 11, 2018

@Franco992 Bonus materials 😄 Here is PR that enable SDL schemas to be executable:
#469
Also graphql-tools wrapper was announced in the same PR 😉

@Franco992

This comment has been minimized.

Copy link
Author

Franco992 commented Jun 13, 2018

@IvanGoncharov Thanks for your clarification.

You are right - I was too focused on the SDL approach. The manual creation supports to assign resolvers. You are absolutely right.

Personaly I think it should be possible to attach resolvers to SDL schema but I don't think we should replicate makeExecuteableSchema design.

What would be the difference? Do you have any information on this (Didn't find it in the link)?

On question that goes around in my mind:
Is it safe to say (in general for all graphql spec compliant implementations, independent of programming language, ...), that there has to be a resolver function for every field?

Is that true? Can I use this formulation? So is this a need of a implementation? I ask because the specified execution model was always a big advantage of GraphQL for me (query language + runtime in the spec = almost same behaviour on different platform / programming languages).

Thank you very much and sorry for annoying you.

@IvanGoncharov IvanGoncharov added this to the v15.0.0 milestone Jan 31, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment