graphql-let icon indicating copy to clipboard operation
graphql-let copied to clipboard

API design of Resolver Types feature is problematic/misleading

Open marklawlor opened this issue 4 years ago • 8 comments

When using the Resolver Types feature it is adding the output to all files including the document types. One of the features of Resolver Types is the ability to use relative paths to external types. As documents can be anywhere within the project (most likely near the component) these relative paths break.

As the Resolver Types are only used on the server, I believe they should only be included in the schema types file.

A workaround is to set a baseUrl in your tsconfig and use paths relative to it

marklawlor avatar May 15 '20 07:05 marklawlor

Sorry, I couldn't get it. Resolver Types feature only effects, say, schema.graphqls.d.ts. Could you describe a brief example of the problematic case?

piglovesyou avatar May 17 '20 14:05 piglovesyou

So the issue was that I had typescript-resolvers in my plugin list, I missed the line in the documentation where you should not include it, just have it installed.

I think that's possibly a problematic API design as tools rarely work in that manner (for example both babel and eslint have moved away from auto including installed plugins).

Could there possible be another parameter in the .graphql-let.yaml for schema/document plugins? eg

plugins: // Shared by both schema and documents
schemaPlugins: // schema only documents
documentPlugins: // document only plugins (not sure if this one is needed, but it might speed up server compile time if its not loading unneeded plugins?)

marklawlor avatar May 17 '20 23:05 marklawlor

Thanks for the explanation, I think I get it. Then the title of the issue would be:

  • API design of Resolver Types feature is problematic/misleading
  • Codegen plugins for schema is unable to configure

I think I'll modify it unless you disagree.

piglovesyou avatar May 19 '20 13:05 piglovesyou

Agreed - updated the title.

marklawlor avatar May 21 '20 23:05 marklawlor

Your idea is reasonable, the explicit config is preferable.

@marklawlor Let me ask a question, is plugins as a shared plugin list helpful? It seems codegen.yml doesn't have such a feature and the order of the plugins goes uncontrollable if a config has both a shared and a schema/document specific list.

I think deprecating plugin field and support only schemaPlugins and documentPlugins is our option.

piglovesyou avatar Jul 18 '20 16:07 piglovesyou

The more I use graphql-codegen and graphql-let, the most I wished the configs converged into a similar format which would be a single plugins list.

So my opinion on this issue has changed and I think it should be:

  • To use typescript-resolvers you need to set baseUrl in your tsconfig and your mapped typeds need a url relative to the baseUurl (been using this, works fine just needs documentation)
  • typescript-resolvers needs to be declared in plugins. Maybe it can be hard-coded to be ignored for documents.

I used to think that it would be good for the schema and document types to be kept separate. But they are not checked in to the repo, and they are not used in production. So I'm not sure if I actually care about it anymore.

marklawlor avatar Jul 19 '20 01:07 marklawlor

@marklawlor Sorry, I couldn't understand how modifying baseUrl affects typescript-resolver issue. Could you tell me a little more?

And you're saying it's okay to run them for both schema and document since the output is just temporary files.

plugins:
  - typescript
  - typescript-operations
  - typescript-react-apollo
  - typescript-resolvers

I saw someone who cares consistency and efficiency even in temporary outputs.. I'm skeptical as well. if something is unnecessary, it's better not to exist.

piglovesyou avatar Jul 19 '20 02:07 piglovesyou

I here,

In my case, I would love to be able to configure typescript-resolver plugin.
In many projects, I use type resolvers because the result of the query is not the expected type.
To handle typing in those cases, I use the mapper configuration of the plugin.

Supporting plugins configuration in this project would be awesome. graphql-let would become just a superset of graphql-codegen, without any restriction. But I understand it is not as easy as saying it.

guillaumeLamanda avatar Dec 30 '20 13:12 guillaumeLamanda