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

chore(docs): Add GraphQL Typegen #35584

Merged
merged 16 commits into from
May 24, 2022
Merged

chore(docs): Add GraphQL Typegen #35584

merged 16 commits into from
May 24, 2022

Conversation

LekoArts
Copy link
Contributor

@LekoArts LekoArts commented May 4, 2022

Description

Adds documentation for GraphQL Typegen and adds an example using-graphql-typegen.

TODOs:

  • ESLint plugin
  • Schema customization
  • Update examples

Documentation

  • New page "GraphQL Typegen" - Rendered Version
  • Linking to existing TypeScript page

Related Issues

#35420

[ch50091]

@gatsbot gatsbot bot added the status: triage needed Issue or pull request that need to be triaged and assigned to a reviewer label May 4, 2022
@LekoArts LekoArts added type: documentation An issue or pull request for improving or updating Gatsby's documentation and removed status: triage needed Issue or pull request that need to be triaged and assigned to a reviewer labels May 4, 2022
@LekoArts LekoArts marked this pull request as draft May 4, 2022 10:29
@LekoArts LekoArts added this to the GraphQL TypeScript Generation milestone May 4, 2022
@LekoArts LekoArts changed the title chore(docs): Add GraphQL Codegen chore(docs): Add GraphQL Typegen May 4, 2022
@LekoArts LekoArts marked this pull request as ready for review May 10, 2022 14:06
@Urigo
Copy link

Urigo commented May 11, 2022

Very excited about this @LekoArts !
Please let us know if we could help/support you somehow!

Do you think it would be helpful to mention somewhere that it uses the Codegen library under the hood or it just unnecessary for the users?

also I haven't got too deep to the implementation yet but did you get to look at TypedDocumentNode and the RFC from @cometkim ?
Would love to hear your thoughts on that approach

One last thing, here I would change from the unmaintained eslint-plugin-graphql to graphql-eslint

@LekoArts
Copy link
Contributor Author

LekoArts commented May 11, 2022

Hi @Urigo 👋

Please let us know if we could help/support you somehow!

I think what I'm still unsure about this how the VSCode GraphQL Plugin experience can come to other IDEs like emacs, vim, etc. -- how we'd document this. We generate a GraphQL config for this plugin so others that only implement a GraphQL language server should be able to reuse it? But I haven't found anything actionable yet via Google.

Do you think it would be helpful to mention somewhere that it uses the Codegen library under the hood or it just unnecessary for the users?

My stance is that as long as we don't expose public APIs that would interact with it it's not necessary to mention. I could write a conceptual guide about the feature (https://www.gatsbyjs.com/docs/conceptual/) which would be a good place to mention it. (As reference: https://www.gatsbyjs.com/docs/conceptual/image-plugin-architecture/)

also I haven't got too deep to the implementation yet but did you get to look at TypedDocumentNode and the RFC from @cometkim ?

We thought about this internally already prior to the RFC and long-term it's what we probably want to do. But it's not something we can do just now. When we revisit our query APIs we'll definitely keep that in mind. So this would be like a v2 of this typegen feature because it would be really neat to not have to import the types.

One last thing, here I would change from the unmaintained eslint-plugin-graphql to graphql-eslint

I intend to deprecate the plugin and link people to this now built-in feature, but good callout as I should add this to the docs that you can use it. Thanks!

@cometkim
Copy link
Contributor

cometkim commented May 11, 2022

Inferred fields such as site metadata are nullable by default, and users may wonder if it is possible to make them non-nullable.

It would be nice to provide a simple example of adding a non-null assertion via schema customization and a link to the advanced guide to the schema customization.

exports.createSchemaCustomization = ({ actions }) => {
  actions.createTypes(`
    type Site {
      siteMetadata: SiteSiteMetadata! # This breaks build if siteMetadata is not provided
    }

    type SiteSiteMetadata {
      title: String! # This breaks build if title is not provided
  `);
};

@LekoArts
Copy link
Contributor Author

LekoArts commented May 20, 2022

@Urigo When trying to use the ESLint plugin I run into this:

Bildschirmfoto 2022-05-20 um 16 45 20

Seems like dimaMachina/graphql-eslint#292 but I can't solve it. I added it here: cf3ccd8

@dimaMachina
Copy link
Contributor

@LekoArts Hi there! I am the maintainer of graphql-eslint and I'm here to help you. It's strange but I don't have your error output. Instead, when I run yarn lint I have:

 dmytro@MBP-Dmytro  ~/Desktop/gatsby/examples/using-graphql-typegen   docs/typegen-two  yarn lint
yarn run v1.21.0
$ eslint . --ignore-path .gitignore --ext .ts,.tsx

/Users/dimitri/Desktop/gatsby/examples/using-graphql-typegen/src/pages/404.tsx
  40:10  error  'process' is not defined  no-undef

/Users/dimitri/Desktop/gatsby/examples/using-graphql-typegen/src/pages/index.tsx
  12:40  error  'Queries' is not defined  no-undef

✖ 2 problems (2 errors, 0 warnings)

error Command failed with exit code 1.

When I change your lint command to be eslint --ignore-path .gitignore . I receive another output:

 ✘ dmytro@MBP-Dmytro  ~/Desktop/gatsby/examples/using-graphql-typegen   docs/typegen-two  yarn lint
yarn run v1.21.0
$ eslint --ignore-path .gitignore .

/Users/dimitri/Desktop/gatsby/examples/using-graphql-typegen/.eslintrc.js
  1:1  error  'module' is not defined  no-undef

/Users/dimitri/Desktop/gatsby/examples/using-graphql-typegen/graphql.config.js
  1:1   error  'module' is not defined   no-undef
  1:18  error  'require' is not defined  no-undef

/Users/dimitri/Desktop/gatsby/examples/using-graphql-typegen/src/components/info.tsx
  0:0  error  Parsing error: [graphql-eslint] Cannot find module './.cache/typegen/graphql.config.json'
Require stack:
- /Users/dimitri/Desktop/gatsby/examples/using-graphql-typegen/graphql.config.js
- /Users/dimitri/Desktop/gatsby/examples/using-graphql-typegen/node_modules/cosmiconfig/dist/loaders.js
- /Users/dimitri/Desktop/gatsby/examples/using-graphql-typegen/node_modules/cosmiconfig/dist/ExplorerBase.js
- /Users/dimitri/Desktop/gatsby/examples/using-graphql-typegen/node_modules/cosmiconfig/dist/Explorer.js
- /Users/dimitri/Desktop/gatsby/examples/using-graphql-typegen/node_modules/cosmiconfig/dist/index.js
- /Users/dimitri/Desktop/gatsby/examples/using-graphql-typegen/node_modules/graphql-config/index.js
- /Users/dimitri/Desktop/gatsby/examples/using-graphql-typegen/node_modules/@graphql-eslint/eslint-plugin/index.js
- /Users/dimitri/Desktop/gatsby/examples/using-graphql-typegen/node_modules/@eslint/eslintrc/dist/eslintrc.cjs

/Users/dimitri/Desktop/gatsby/examples/using-graphql-typegen/src/pages/404.tsx
  40:10  error  'process' is not defined  no-undef

/Users/dimitri/Desktop/gatsby/examples/using-graphql-typegen/src/pages/index.tsx
   0:0   error  Parsing error: [graphql-eslint] Cannot find module './.cache/typegen/graphql.config.json'
Require stack:
- /Users/dimitri/Desktop/gatsby/examples/using-graphql-typegen/graphql.config.js
- /Users/dimitri/Desktop/gatsby/examples/using-graphql-typegen/node_modules/cosmiconfig/dist/loaders.js
- /Users/dimitri/Desktop/gatsby/examples/using-graphql-typegen/node_modules/cosmiconfig/dist/ExplorerBase.js
- /Users/dimitri/Desktop/gatsby/examples/using-graphql-typegen/node_modules/cosmiconfig/dist/Explorer.js
- /Users/dimitri/Desktop/gatsby/examples/using-graphql-typegen/node_modules/cosmiconfig/dist/index.js
- /Users/dimitri/Desktop/gatsby/examples/using-graphql-typegen/node_modules/graphql-config/index.js
- /Users/dimitri/Desktop/gatsby/examples/using-graphql-typegen/node_modules/@graphql-eslint/eslint-plugin/index.js
- /Users/dimitri/Desktop/gatsby/examples/using-graphql-typegen/node_modules/@eslint/eslintrc/dist/eslintrc.cjs
  12:40  error  'Queries' is not defined                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                             no-undef

✖ 7 problems (7 errors, 0 warnings)

So and finally to make it works I made yarn develop and in another terminal window yarn lint

 ✘ dmytro@MBP-Dmytro  ~/Desktop/gatsby/examples/using-graphql-typegen   docs/typegen-two ±  yarn lint
yarn run v1.21.0
$ eslint --ignore-path .gitignore .

/Users/dimitri/Desktop/gatsby/examples/using-graphql-typegen/.eslintrc.js
  1:1  error  'module' is not defined  no-undef

/Users/dimitri/Desktop/gatsby/examples/using-graphql-typegen/graphql.config.js
  1:1   error  'module' is not defined   no-undef
  1:18  error  'require' is not defined  no-undef

/Users/dimitri/Desktop/gatsby/examples/using-graphql-typegen/src/pages/404.tsx
  40:10  error  'process' is not defined  no-undef

/Users/dimitri/Desktop/gatsby/examples/using-graphql-typegen/src/pages/index.tsx
  35:10  error  Field `site.id` must be selected when it's available on a type.
Include it in your selection set or add to used fragment `SiteInformation`  @graphql-eslint/require-id-when-available
  12:40  error  'Queries' is not defined                                                                                                                    no-undef

✖ 6 problems (6 errors, 0 warnings)

error Command failed with exit code 1.

Co-authored-by: Dimitri POSTOLOV <[email protected]>
@LekoArts
Copy link
Contributor Author

Thanks @B2o5T 👍 I got it working now, too

Copy link
Contributor

@Khaledgarbaya Khaledgarbaya left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, just 2 minor comments.
Did you consider adding a GIF or a video about how the feature is working? is this something we usually add our docs?


If you're already using [Gatsby with TypeScript](/docs/how-to/custom-configuration/typescript) and manually typing the results of your queries, you'll learn in this guide how Gatsby's automatic GraphQL Typegen feature can make your life easier. By relying on the types that are generated by Gatsby itself and using autocompletion for GraphQL queries in your IDE you'll be able to write GraphQL queries quicker and safer.

This feature was added in `[email protected]`.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should this be

Suggested change
This feature was added in `[email protected]`.
> This feature was added in `[email protected]`.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We use the quote here and there to note this, but always under a heading. I felt that this was fine to add it as it's own sentence here.

But we don't have a style guide for this really

`
```

It is important that your query has a name (here: `query TypegenPage {}`) as otherwise the automatic type generation doesn't work. We recommend naming the query the same as your React component and using [PascalCase](https://en.wiktionary.org/wiki/Pascal_case). You can enforce this requirement by using [`graphql-eslint`](#graphql-eslint).
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

extra space to remove here

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I intentionally indented it here as it refers to the code block above

@imjoshin
Copy link
Contributor

Did you consider adding a GIF or a video about how the feature is working? is this something we usually add our docs?

PR looks good to me, but a GIF as @Khaledgarbaya suggested would be great for evangelizing the feature in our docs. Happy to ✅ if you feel otherwise. 😄

@LekoArts LekoArts merged commit 1c392e6 into master May 24, 2022
@LekoArts LekoArts deleted the docs/typegen-two branch May 24, 2022 06:15
@LekoArts
Copy link
Contributor Author

We'd want to have a video as you then can have subtitles, too, which makes it more accessible than a GIF.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: documentation An issue or pull request for improving or updating Gatsby's documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants