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

[RFC] Typed GraphQL #418

Closed
mohsen1 opened this issue Apr 11, 2020 · 10 comments · Fixed by #2485
Closed

[RFC] Typed GraphQL #418

mohsen1 opened this issue Apr 11, 2020 · 10 comments · Fixed by #2485

Comments

@mohsen1
Copy link
Contributor

mohsen1 commented Apr 11, 2020

There is interest in making Redwood GraphQL operations type-safe. Since Redwood is full-stack, we can offer a much better integration with code-generators and React components than existing solutions.

Core of my proposal is that type code generation and usage is invisible to the user.

State of the art

Currently using code generators such as Apollo or graphql-code-generator requires user to setup a watcher that watches graphql schema and existing query/mutation files, then it writes down resulting generated code. The second step is to import those generated types to make GraphQL operations type safe.

import { useQuery } from 'react-apollo';

import GetUserProfile from './GetUserProfile.graphql'

import { GetUserProfile as IGetUserProfile, GetUserProfileVariables } from './__generated__/GetUserProfile.ts';

const { data } = useQuery<IGetUserProfile, GetUserProfileVariables>(GetUserProfile);

oof! So much boilerplate and importing just to get that data typed properly.

The graphql-code-generator typescript-react-apollo offers generating code that includes those imports so using the GetUserProfile query is as simple as following

import { useGetUserProfileQuery } from './__generated__'

This hook is strongly typed and the resulting data will have proper typing.

The generated code is living alongside of the source code which makes code structure extremely polluted.

Proposal

The code generation should be done as part of Redwood dev server process and be completely invisible to the user.

Put generated code in node_modules

Learning from Prisma, I think this generated code can go straight into @redwood/web so we can avoid team debates on "should we check in the generated code or not?"!!

All of generated code can live under /types directory

Use TypeScript Namespaces

This will allow access to deeper interfaces using . instead of typical bracket notation.

GetBlogPosts.Success looks better than GetBlogPosts['Success']

Example

We can imagine generating a Cell for BlogPosts from the tutorial would look like:

export const QUERY = gql`
  query GetBlogPosts {
    blogPosts {
      id
    }
  }
`
import { GetBlogPosts } from '@redwood/web/types';

export const Loading = () => <div>Loading...</div>

export const Empty = () => <div>Empty</div>

export const Failure: GetBlogPosts.Failure = ({ error }) => <div>Error: {error.message}</div>

export const Success: GetBlogPosts.Success = ({ blogPosts }) => {
  return JSON.stringify(blogPosts)
}

Notes about example

  • Query is named. This helps the codegen generated better looking types. Query names must be unique to avoid name collision.
  • Only Success and Failure are typed. No reason to type Empty and Loading. However they better be React.FC type.
  • Import is coming after definition of the QUERY. I think this is useful because it implies that types are result of query. This might break linting and we decide it is not worth it.

Implementation details

Work to do this requires changes in the core and default template. Why the template is not in the monorepo?

graphql-code-generator

We would probably require to write a custom plugin for graphql-code-generator tailored to Redwood use case.

Possible challenges

  • Renaming/Deleting queries: codegens are terrible at cleaning up after themselves
  • TypeScript Language server heavily caching node_modules types and editors not picking up newly generated types in the node_moduels. @nikolasburk and @imsuchanek did you have this problem in Prisma?
@olance
Copy link
Contributor

olance commented Apr 11, 2020

Query names must be unique to avoid name collision.

Any chance we could mitigate this with some namespacing of the queries?
Not sure how much of a pain avoiding those collisions would be, but if we could have something like a BlogPosts.GetPosts query, that'd allow more generic query names to be used without giving it too much thoughts.

@mohsen1
Copy link
Contributor Author

mohsen1 commented Apr 11, 2020

@olance good idea! I like that. Maybe even deeper like Cells.BlogPosts.GetPorsts

@peterp
Copy link
Contributor

peterp commented Apr 14, 2020

The code generation should be done as part of Redwood dev server process and be completely invisible to the user.

I can understand why you would like to place it into the dev-server process, but I don't think it's the correct place to integrate because in my mind the api-dev-server is only concerned with serving the serverless functions.

I will eventually rename it to api-server so that people can self-host.

At the moment it only deals with aws lambda style functions, but it is architected to support additional environments. (My pending PR that adds support for additional sides makes this clearer.)

I think we should include an additional concept in between the CLI (yarn rw dev api) and the dev-server: a file watcher that runs a bunch of specified functions in response to a file changing.

This will give us a natural place to include something like this (and another place for others to include something like this too), separates the concerns, and let's us think about graphql-generation in a separated way.

@Rosenberg96
Copy link
Contributor

Hi @mohsen1 really excited to see this issue and I believe this is exactly the great out-of-the-box developer experience Redwood offers very well.

Put generated code in node_modules
Learning from Prisma, I think this generated code can go straight into @redwood/web so we can avoid team debates on "should we check in the generated code or not?"!!

To your above comment, I know this approach by the Prisma team triggered some mixed emotions on HN.

In this context, I think it makes sense to follow the same principle as Prisma, where even if the generated types are generated in the node_modules we should allow users to specify their preferred output directory, since some teams will opt for the versioned types generated (I'm one of them).

Do you have any thought on the following point?

  1. Currently, the full schema.graphql is not persisted as a file (AFAIK), but is currently generated from the importAllMacro.js and passed directly to the ApolloServer. Should we persist the full schema in a schema.graphql file or even from the running instance?

Im inclined to opt for the file to be generated since it allows us to analyse this is on changes, which i find extremely powerful - see GraphQL Inspector

@aldonline
Copy link
Contributor

aldonline commented Apr 28, 2020

Functionally, I believe this is a good proposal and should be considered.

But I will focus on a tangential discussion that emerges from this issue: TypeScript and Code Generation

(There is a previous discussion around TypeScript and Redwood in general, but assuming that gets sorted out, then the code generation discussion will immediately follow)

TypeScript and Code Generation

  • TypeScript does not include code-generation facilities.
  • There are different flavors of code generation that are relevant to this feature, of which TypeScript supports none (as of today): Type Providers, Macros, Compiler Lifecycle Hooks
  • This means that all code generation efforts in the community are ad-hoc. Also, the general architecture of these code generators is all over the place, as evidenced by questions such as:
    • When should I run code generation?
    • Where do I store the generated files?
    • Should I check-in generated code?
  • It would definitely be worth the time to try and see if there is anything the TypeScript team is planning to do to support this that isn't in the official roadmap. @mojombo: This is something that we should bring up if we ever meet with them. I would believe that with the amount of ad-hoc code generation projects out there, and the traction behind babel.macros, the TypeScript team must be considering looking into this in the near future.

Now, with this in mind, let's look at some of the issues:

Where to store generated files?

  • Our experience, and something others have mentioned in this thread already, is that storing things in node_modules breaks some assumptions built into the TypeScript language server. This means that changes are not easily picked up by IDEs, which breaks the DX.
  • The workaround is to store generated types in one of the source directories that TypeScript "believes" are user-written code.
  • Some alternatives are being suggested in this thread already, so I'll just add one more:
    • Because TypeScript now supports multiple rootDirs or paths, you can have a "src" tree dedicated just for generated code, which doesn't pollute the user space. For example:
* src
  * components
    * MyCell
      * MyCell.ts
* src-gen
  * components
    * MyCell
      * types.ts
  • This "overlay" structure makes it easy to import generated code and provides natural namespacing. Of course, this only makes sense for generated code that is related to a particular file. For cases where generated symbols must be used globally (ex: named routes), you could create a file at the top of src-gen with a global declaration. The code in this folder is usually type-related only. Runtime code/libraries are still injected with webpack.
  • Whether to check-in the src-gen tree or not is for the user to decide.

When to run code-generation?

  • Once more, because of TypeScript's lack of native support for code generation, there is no clear moment to run code generators.
  • A language like F#, for example, has "Type Providers", which are implemented by library authors but ultimately executed by the compiler at various points in the development lifecycle.
  • The current status quo for code generation in languages that don't provide the right hooks is to have a special "code-generation" task that runs on watch mode and is usually started alongside the dev server.

From the discussion:

  • @mohsen1 suggests to add this to the dev-server. Which I interpret to be just a way of saying that this is something that should run in the background when the user is developing
  • @peterp says that the "dev-server" is a runtime component, so that is not the right place to put it. He is right, but they're talking about different things.
  • The correct answer is that the code-generation process should run as a separate task. It is part of the build process, but it runs on "watch" mode.
  • Note: @mohsen1 points out that cleanup is usually an issue with code generation. He is right. This is because it is easy to write a code generator, but adding the ability to keep track of all that was generated (so it can be cleaned up when it is no longer relevant) is hard. A solution I personally like to this problem is to keep the code generation script "dumb" (it doesn't need to keep track of things) but make it add some metadata to all generated files so they can be tracked back to their sources. This metadata can be as simple as a comment line with the original filename (you could also use full sourcemaps here, but that's a lot of work). With this info you can write a very simple GC process that deletes generated files whose sources no longer exist. The takeaway from this is: @peterp Do not worry too much about building a code generator that knows how to cleanup after itself. We can add that feature later.

  • Conclusion: Code generation is a separate task (similar to "compile"). It should be started by the dev server, but it is not part of the dev server itself.

@aldonline aldonline mentioned this issue Apr 28, 2020
17 tasks
@petrbela
Copy link

Have you guys seen https://gqless.dev/? It basically skips the entire manual process of writing queries and instead "automagically" generates them (along with TypeScript definitions) behind the scenes from the fields a component actually uses. It doesn't seem to be fully complete yet (e.g. mutations) but I think it shows a pretty elegant way of reducing boilerplate in RoR's spirit.

@peterp
Copy link
Contributor

peterp commented Sep 14, 2020

@petrbela I really like the gqless.dev project! I've been thinking a lot about it and would love a way to simplify fetching data from the GraphQL server in the long term, in the short term, I think having generated types is a really decent way forward.

@mohsen1
Copy link
Contributor Author

mohsen1 commented Sep 14, 2020

Types help the discovery of field in the response. If we go from usage of the response to types we lose that nice discoverability

@peterp
Copy link
Contributor

peterp commented Apr 6, 2021

We've added the GraphQL VSCode plugin which works very well during development time - I think we can improve it by dumping the SDL to the disk instead of requiring the api-server to be running.

  • GQL is typed

What we often want is the ability to have types for the Success method in a cell.

@dac09
Copy link
Contributor

dac09 commented May 21, 2021

For anyone still tracking this #2485 is up that implements this (finally!) and has a bunch of other nice things to go along with it.

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

Successfully merging a pull request may close this issue.

8 participants