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

Feedback on Resolver Types #578

Open
marklawlor opened this issue Aug 18, 2021 · 3 comments
Open

Feedback on Resolver Types #578

marklawlor opened this issue Aug 18, 2021 · 3 comments

Comments

@marklawlor
Copy link

Feedback

I've been using this library with the experimental resolver types for a while now but as my application grows I'm finding this feature doesn't quite work as you want it too and becomes a burden to use

The problem is that graphql-let shares its types across both the front-end & back-end however in practice these are often different, especially for custom scalars and enums with interval values.

Custom scalars

Custom scalars that transform their input/output will always need type casing.

Lets say you want to use a custom Date scalar. The typing for this should be string on the front-end and Date on the back-end, but due to the reused types you can to pick only 1. This starts to be quite annoying with mutations and/or query arguments where every resolver starts with you cast inputs into the correct values and/or you need to cast the frontend's input variables.

Enums

Enum's are trickier and have caused us a number of runtime errors!

If you follow the enum with interval values example, your front-end queries will now be typed to use the internal value. If a developer is not aware of this, then the application will compile but fail at runtime with an incorrect enum value.

It cannot be turned off

I thought the best workaround for this problem would be to stop generate resolver types with graphql-let and just do it manually with graphql-code-generator, but there is no way to disable the feature if you have @graphql-codegen/typescript-resolvers installed.

Proposed solutions

Have a config option to opt of out Resolver Types

Probably the easiest, but I would like to continue to use the library to generate these types.

Generate separate types for front-end & back-end.

graphql-code-generator solves this by allowing you to specify the generates value. You stated before that you won't want this feature, but I think you should to reconsider this stance as it makes graphql-let difficult to use on larger applications.

@piglovesyou
Copy link
Owner

piglovesyou commented Sep 11, 2021

@marklawlor First, I really appreciate this comprehensive, critical feedback about server-side code generation. Let us now discuss the short-term solution first, although I'm really interested in discussing whether we can share generated code between the server-side and client-side.

I think this #578 and #600 (by @cbmd) point out a common problem: inflexibility to configure schema-side options (not document-side). Currently, graphql-let has poor room to accept user options for schema-side; it loads the Resolver Types plugin automatically, and it doesn't accept plugin options from users.

Perhaps it's our good time to introduce a new configuration for schema-side to be more explicit and accept user options in .graphql-let.yml

  schema: ...
  documents: ...
  plugins: ... # for docuemnt-side
  config: ... # for docuemnt-side
+ typePlugins: # for schema-side
+     - typescript
+     - resolver-types
+ typePluginConfig: # for schema-side
+     - typesPrefix: ''

@marklawlor, I've had #96 too in my mind. Resolver Types has been an experimental feature, but it would be not anymore once it becomes explicit like the above, I guess. How does it sound, guys?

@piglovesyou
Copy link
Owner

Partially related to #479; the issue only wants typePlugins to run.

@marklawlor
Copy link
Author

  schema: ...
  documents: ...
  plugins: ... # for docuemnt-side
  config: ... # for docuemnt-side
+ typePlugins: # for schema-side
+     - typescript
+     - resolver-types
+ typePluginConfig: # for schema-side
+     - typesPrefix: ''

I think this looks good, but I'd adjust the config names slightly

One thing to point out, the client and server don't need to share types, they just need their definitions to be kept in sync. With maybe enums as the exception.

For example, I am building an API using the Relay Specification which uses an ID scalar which transforms a database number to an opaque string. The client types for ID will be string and the types on the server will be number. Because of this simple change, a large amount of the type definitions cannot be shared between client & server.

Internally, I think graphql-let needs to generate a __types__/client.d.ts and a __types__/server.d.ts if serverPlugins are used. Maybe __types__/enums as enums have a separate run time component, as discussed here

I have used the terms client / server instead of frontend / backend as maybe the client is another backend? I also would avoid using document/schema as why correct I don't think they are friendly to new developers and client/server are simply more familiar.

So I would rename the config:

  schema: ...
  documents: ...
  plugins: ... # for client/document side
  config: ... # for client/document side
+ serverPlugins: # for server/schema side
+     - typescript
+     - resolver-types
+ serverPluginConfig: # for server/schema side
+     - typesPrefix: ''

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

2 participants