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

[templates/nextjs] [templates/nextjs-styleguide] Remove graphql-let and rely directly on graphql-codegen for graphql code generation #1713

Open
wants to merge 9 commits into
base: dev
Choose a base branch
from
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,10 @@ Our versioning strategy is as follows:
* Upgrade to Node.js 20.x ([#1679](https://github.com/Sitecore/jss/pull/1679))([#1681](https://github.com/Sitecore/jss/pull/1681))
* `[nextjs/template]` Upgrade graphql-codegen packages to latest ([#1711](https://github.com/Sitecore/jss/pull/1711))

### 🛠 Breaking Changes

* `[templates/nextjs]` `[templates/nextjs-styleguide]` Remove graphql-let and rely directly on graphql-codegen for graphql code generation. ([#1713](https://github.com/Sitecore/jss/pull/1713))

## 21.6.0

### 🎉 New Features & Improvements
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
overwrite: true
documents: 'src/**/*.graphql'
schema: 'src/temp/GraphQLIntrospectionResult.json'
generates:
src/temp/graphql-types.ts:
plugins:
- 'typescript'
- 'typescript-operations'
- 'typescript-react-query'
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import {
resetEditorChromes
} from '@sitecore-jss/sitecore-jss-nextjs/utils';
import NextLink from 'next/link';
import { ConnectedDemoQueryDocument } from './GraphQL-ConnectedDemo.dynamic.graphql';
import { ConnectedDemoQueryDocument } from 'graphql-types';
import {
<%- helper.getAppPrefix(appPrefix, appName, false) %>AppRoute as AppRoute,
Item,
Expand Down

This file was deleted.

5 changes: 0 additions & 5 deletions packages/create-sitecore-jss/src/templates/nextjs/gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,6 @@
/.next*/
/out/

# graphql code generation
/.generated
*.graphql.d.ts
*.graphqls.d.ts

# misc
.DS_Store

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
overwrite: true
# documents: 'src/**/*.graphql'
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you add a comment explaining why is this commented out, and when devs might need to enable it?

Copy link
Contributor

Choose a reason for hiding this comment

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

@yavorsk If it will be disabled by default, it means that in case we scaffold nextjs-styleguide - graphql code generation will not be executed, so the error will be thrown in GraphQL-ConnectedDemo, did you test that?

Copy link
Contributor

Choose a reason for hiding this comment

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

@yavorsk I see, so you duplicated a config file, is it possible to override npm command instead by providing documents flag? If it's possible. In order to don't copy/paste the config

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The library does not provide documents flag. There is a config flag, which specifies the config file to be used, but I am not sure if that would be cleaner in our case...?

schema: 'src/temp/GraphQLIntrospectionResult.json'
generates:
src/temp/graphql-types.ts:
plugins:
- 'typescript'
- 'typescript-operations'
- 'typescript-react-query'
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@
"@graphql-codegen/typed-document-node": "^5.0.1",
"@graphql-codegen/typescript": "^4.0.1",
"@graphql-codegen/typescript-operations": "^4.0.1",
"@graphql-codegen/typescript-react-query": "^6.0.0",
"@graphql-codegen/typescript-resolvers": "^4.0.1",
"@graphql-typed-document-node/core": "^3.2.0",
"@sitecore-jss/sitecore-jss-cli": "~21.7.0-canary",
Expand All @@ -49,6 +50,7 @@
"@types/react-dom": "^18.0.5",
"@typescript-eslint/eslint-plugin": "^5.49.0",
"@typescript-eslint/parser": "^5.49.0",
"@tanstack/react-query": "^5.17.9",
Copy link
Contributor

Choose a reason for hiding this comment

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

Where is this dependency used?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it is referenced by the generated file

Copy link
Contributor

Choose a reason for hiding this comment

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

@yavorsk Should you use this package instead?

Copy link
Contributor

Choose a reason for hiding this comment

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

It's still referenced 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.

the two packages are different: @graphql-codegen/typescript-react-query is a plugin used during code generation; @tanstack/react-query is referenced by the generated ts file;

"chalk": "~4.1.2",
"chokidar": "~3.5.3",
"constant-case": "^3.0.4",
Expand All @@ -60,7 +62,6 @@
"eslint-plugin-prettier": "^4.2.1",
"eslint-plugin-react": "^7.32.1",
"eslint-plugin-yaml": "^0.5.0",
"graphql-let": "^0.18.6",
"npm-run-all": "~4.1.5",
"prettier": "^2.8.3",
"ts-node": "^10.9.1",
Expand All @@ -71,7 +72,7 @@
"scripts": {
"jss": "jss",
"lint": "eslint ./src/**/*.tsx ./src/**/*.ts ./scripts/**/*.ts",
"bootstrap": "ts-node --project tsconfig.scripts.json scripts/bootstrap.ts && graphql-let",
"bootstrap": "ts-node --project tsconfig.scripts.json scripts/bootstrap.ts && jss graphql-codegen",
Copy link
Contributor

Choose a reason for hiding this comment

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

would it be better to use npm graphql-codegen here? jss is just an alias for npm.
Or better yet - is it possible to add graphql init logic into bootstrap script?

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 left it here because I think it would be easier for the user to disable code generation at build time if needed. Do you think it would be better to be in the script?

"build": "npm-run-all --serial bootstrap next:build",
"graphql:update": "ts-node --project tsconfig.scripts.json ./scripts/fetch-graphql-introspection-data.ts",
"next:build": "next build",
Expand All @@ -81,6 +82,7 @@
"start:connected": "npm-run-all --serial bootstrap --parallel next:dev start:watch-components",
"start:production": "npm-run-all --serial bootstrap next:build next:start",
"start:watch-components": "ts-node --project tsconfig.scripts.json scripts/generate-component-builder/index.ts --watch",
"install-pre-push-hook": "ts-node --project tsconfig.scripts.json ./scripts/install-pre-push-hook.ts"
"install-pre-push-hook": "ts-node --project tsconfig.scripts.json ./scripts/install-pre-push-hook.ts",
"graphql-codegen": "graphql-codegen --config graphql-codegen.yml"
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,13 @@ const graphqlPlugin = (nextConfig = {}) => {
config.module.rules.push({
test: /\.graphql$/,
exclude: /node_modules/,
use: [options.defaultLoaders.babel, { loader: 'graphql-let/loader' }],
use: [options.defaultLoaders.babel, { loader: 'graphql-tag/loader' }],
});

config.module.rules.push({
test: /\.graphqls$/,
exclude: /node_modules/,
use: ['graphql-let/schema/loader'],
use: ['graphql-tag/loader'],
});

config.module.rules.push({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
"lib/*": ["src/lib/*"],
"temp/*": ["src/temp/*"],
"assets/*": ["src/assets/*"],
"graphql-types": ["node_modules/@types/graphql-let/__generated__/__types__"],
"graphql-types": ["src/temp/graphql-types.ts"],
"react": ["node_modules/react"]
},
"target": "es5",
Expand Down