Add experimental codegen support to CLI

[frandiox]

Closes #568
Related #887

Updated description: #707 (comment)

This PR uses codegen to generate types for every GraphQL operation in the user app and augment the storefront.query / storefront.mutate signatures accordingly.

Without codegen

So far we needed to specify the expected return type in a generic, and then we get all the properties from that type back:

With codegen

The returned type is inferred and it only contains the requested properties:

This also works with the types for the {variables: {...}} parameter:

---

Drawbacks

Unfortunately, to implement all of this we need 2 changes in the @graphql-tools/graphql-tag-pluck package:

• The #graphql comment: By default, graphql-tag-pluck only supports leading comments (outside of the query) such as /* GraphQL */ 'query {...}', but not internal comments like '#graphql\n query {...}'. This is just a nice to have since we could probably migrate to leading comments, but I think #graphql is nicer.
• Composing queries with fragments: graphql-tag-pluck automatically removes embedded expressions such as ${MY_FRAGMENT} in query { ...myFragment } ${MY_FRAGMENT} from the generated types, which makes it impossible to match strings in this exploration. The change needed in the package is for keeping the embedded expressions. This is important because we can't support composable queries otherwise.

All these changes are included in a vendor folder so that we can use the code locally, but if we want to support these 2 features properly we should talk with the @graphql-tool folks to add a couple of hooks.

Asking here: ardatan/graphql-tools#5127

---

Tophat

🎩 To test this locally:

1. npm i at the root
2. npm run build:pkg
3. cd templates/hello-world && h2 dev --codegen-unstable
4. Modify the query in root.tsx or other files.

---

Feedback

What kind of feedback I'd like to have:

• Does this feature justify creating and maintaining a new package? (@shopify/hydrogen-codegen)
• Can you think of a simpler way to implement this? Maybe we can find a compromise if we ask the user to change code in their app? There were other explorations made in #291 and #297.
• Should we ask users to run codegen-cli manually? Or should we integrate this in the H2 CLI directly? (e.g. h2 dev --codegen) -- or support both for those who don't use the CLI.
• Test locally in demo-store and see if things work properly for you (there could be cases I haven't considered).

[frehner]

Looks great!

[frandiox]

@frehner I've simplified some of the types and added comments. Can you check if it's more readable now? 🙏

[blittle]

In classic Fran fashion, this PR is 🤯🤯🤯

A few general questions, but I pulled it down and seems to work pretty well!

Is calling this "experimental" OK? Or prefer "unstable"? We are using the latter in utilities but "experimental" felt more correct for commands and flags.

I prefer to stick with the existing unstable nomenclature. I'd like to release more of these features earlier, and have consistent expectations on what "unstable" means. One positive is it's shorter to type as well :)

Should we update the demo-store to use this before releasing this feature? That would ensure everything works but perhaps is too much work and could delay shipping this as unstable/experimental.

I don't think it's necessary, but it would be nice to get the demostore onto it. In my opinion demostore is great for unstable features, but only stable features should be inside the skeleton template.

[blittle]

This is interesting. Is this dup error global to the application, or only to the file?

[frandiox]

To the whole application. It's a limitation of the existing TS-Operation plugin (the codegen plugin that generates types). I think it kind of makes sense since the plugin doesn't know how to name your types otherwise to avoid conflicts. It could append some kind of filepath to them but then it wouldn't be obvious when you want to import them...

[blittle]

Related to my type comment above, the type name LayoutQuery seems to easily collide in the app?

[frandiox]

Well it's up to the developer to change the name of the queries.
Or do you suggest we add a longer suffix to this just in the types? I'm currently just adding ...Query or ...Mutation to it just to follow the same pattern as Fragments (which I think we can't control).

Would you prefer calling them ...GeneratedQuery or similar? Consider also that these types might be imported in-app to specify function parameters (e.g. export function doSomethingWithQueryResult(shop: ShopQuery) {...}) so they shouldn't be too obfuscated.

[blittle]

Hmmm, I'm not sure. It's definitely something we need to make sure is clear in the documentation.

[duncan-fairley]

We're adding ___Query and ___Mutation to the actual queries and mutations but I think adding them to the type names automatically would be nice. I saw that was an option of codegen.

[duncan-fairley]

@frandiox- Sorry for the delay on getting back to you while I was wrapped in Remix conf. Very excited about this and it seems like it will fit in well with (and greatly simplify) our existing projects. Think this will be the most impactful improvement to DX since launch.

The prettier after-write hook is key, great addition.

It sounds from your description like this will support queries and mutations placed in any location in the app (vs only inline in loaders etc)? That's key for us.

[frandiox]

@duncan-fairley Thanks!

It sounds from your description like this will support queries and mutations placed in any location in the app (vs only inline in loaders etc)? That's key for us.

Yeah it works out of the box with any ts/tsx file, and variables don't need to be inlined in the storefront.query(...) function. They can be variables in other files as long as they can be found in ts/tsx files. You might need to add as const to them when using string interpolation, though, but maybe we can add some sort of automatic format/lint for that in the future.

[davidhousedev]

@frandiox Does this handle situations where multiple GraphQL APIs are queried within the same codebase? In our own graphql codegen implementation, we needed to namespace the documents config so it was mutually exclusive between Storefront API and Contentful's GraphQL API

[frandiox]

Does this handle situations where multiple GraphQL APIs are queried within the same codebase? In our own graphql codegen implementation, we needed to namespace the documents config so it was mutually exclusive between Storefront API and Contentful's GraphQL API

I think Codegen will complain when finding queries that don't match the schema so you probably need to keep doing the same: split queries in different files and pass the corresponding globs to each preset. As mentioned at the end of the changeset docs, you can create a <root>/codegen.ts file to do this:

import type {CodegenConfig} from '@graphql-codegen/cli';
import {preset, pluckConfig, schema} from '@shopify/hydrogen-codegen';

export default <CodegenConfig>{
  overwrite: true,
  pluckConfig,
  generates: {
    ['storefrontapi.d.ts']: {
      schema,
      preset,
      documents: ['app/**/*!(.cms).{ts,tsx}'],
    },
    // Your CMS codegen config:
    ['....d.ts']: {
      schema: '...',
      plugins: ['...'],
      documents: ['app/**/*.cms.{ts,tsx}'], // Different documents
    },
  },
};

I'm not sure if there's a better way to do this, open to ideas.

[frandiox]

@davidhousedev

I was thinking about this again. What would you think about something like this?

Basically, this would allow mixing queries from different schemas and we can filter them later checking #graphql:sfapi vs #graphql:<name> 🤔
Not sure if this is desirable or it's better to place them in different files.

[davidhousedev]

Hey @frandiox sorry I missed your response!

There are two considerations here:

1. Codegen of graphql clients for multiple graphql servers
2. IDE feedback and auto-completion for graphql queries and mutations

We're able to get the best case scenario working today by using a single graphql codegen file that uses directory namespacing as you describe in your first suggestion. In case you weren't aware, the codegen config can be referenced within a more standard graphql config file. We use the multi-project config mode to get things working.

I would be surprised if your second suggestion would work with IDE tooling; would non-Hydrogen tools be able to differentiate between graphql queries to Shopify vs other APIs?

[frandiox]

This would work by using Codegen's documentTransform so that should be supported when using the Codegen CLI directly.
Syntax highlight works in VSCode at least but not sure about other features. I don't have many other integrations set up for GraphQL tbh.

Feel free to try it with something like:

['...']: {
  preset,
  schema,
  documents: ['app/**/*.{ts,tsx}'],
  documentTransforms: [
     {transform: ({documents}) => documents.filter(document => document.rawSDL?.includes('#graphql:<name>')}
  ]
}

[davidhousedev]

@frandiox it's tough to say. If VS Code auto completion works with the documentTransform solution for multiple graphql servers then it's an unclear judgement call. We'd probably prefer to use directory namespacing given that's how we've set up everything on our end, but I'll bring this back to our team to see if anyone has another suggestion.