Closed frandiox closed 1 year ago
@frehner I've simplified some of the types and added comments. Can you check if it's more readable now? π
Random thought: is the #graphql
comment preserved and sent over the network to the SFAPI? It's a very small thing, but I wonder if that's why it's not as common to use it that way?
@frehner
Random thought: is the #graphql comment preserved and sent over the network to the SFAPI? It's a very small thing, but I wonder if that's why it's not as common to use it that way?
We don't send it over the network because we minify the query here. We could do this at build time if we get access to build plugins at some point.
Features:
.ts
and .tsx
files when finding a '#graphql...'
inner comment or a leading comment /* Graphql */ '...'
.storefront.query
and storefront.mutation
automatically.variables
property in storefront.query
and storefront.mutation
automatically.Changes:
Added a new @shopify/hydrogen-codegen
package that will be released. For now, this becomes a dependency of @shopify/cli-hydrogen
. We could ask the user to install it instead but I think it's better to do this transparently until it's stable. This package is rather generic and also exports a plugin (not only a preset), which would allow using the same codegen strategy without Hydrogen.
New command h2 experimental-codegen
h2 codegen-unstable
for a standalone one-off generation of all the types. It supports a --watch
flag, which then watches files and prints default logs from the GraphQL Codegen CLI.
Support for codegen in the dev command directly with the flag h2 dev --experimental-codegen
h2 dev --codegen-unstable
. This runs the dev server in the main process and spawns a child process with the codegen watcher logic (to defer resources and also filter logs easier). It filters the logs but we are still able to see errors as warnings (actual text comes from Codegen CLI):
The previous commands automatically read from an optional <root>/codegen.ts
(standard in GQL Codegen CLI), which can be used to tune the type generation. This path can be changed with the flag --codegen-config-path ./another-file.ts
. If files are not found, it uses an internal default configuration which defaults to generate everything in <root>/sfapi.generated.d.ts
.
Running codegen automatically patches the @graphql-tools/graphql-tag-pluck
dependency until https://github.com/ardatan/graphql-tools/issues/5127 is resolved.
Gotchas:
When composing queries with other variables (e.g. injecting fragments) you'll need to add as const
to all the variables to make sure TypeScript infers the correct type instead of just string
:
const AF = `#graphql\n fragment A on X { id }`;
const BF = `#graphql\n fragment B on X { name ...A } ${AF}` as const;
const QUERY = `#graphql\n query Q { X { ...B } } ${BF}` as const;
Perhaps we could add an ESLint/Prettier rule to add as const
when detecting const XYZ = '#graphql ...';
?
Other thoughts / questions
Todos:
We detected some changes in packages/*/package.json
or packages/*/src
, and there are no updates in the .changeset
.
If the changes are user-facing and should cause a version bump, run npm run changeset add
to track your changes and include them in the next release CHANGELOG.
If you are making simple updates to examples or documentation, you do not need to add a changeset.
@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.
@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.
@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
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.
@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.
Hey @frandiox sorry I missed your response!
There are two considerations here:
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?
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>')}
]
}
@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.
Closes #568 Related #887
Updated description: https://github.com/Shopify/hydrogen/pull/707#issuecomment-1547741906
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:#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.graphql-tag-pluck
automatically removes embedded expressions such as${MY_FRAGMENT}
inquery { ...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: https://github.com/ardatan/graphql-tools/issues/5127
Tophat
π© To test this locally:
npm i
at the rootnpm run build:pkg
cd templates/hello-world && h2 dev --codegen-unstable
root.tsx
or other files.Feedback
What kind of feedback I'd like to have:
@shopify/hydrogen-codegen
)h2 dev --codegen
) -- or support both for those who don't use the CLI.