-
Notifications
You must be signed in to change notification settings - Fork 276
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
base: dev
Are you sure you want to change the base?
Changes from 7 commits
0109f53
e5e6587
183b35a
8373ec1
fe81c3d
f0782c8
3ae825d
4b5ed53
948733b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
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' |
This file was deleted.
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 |
---|---|---|
|
@@ -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", | ||
|
@@ -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", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Where is this dependency used? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. it is referenced by the generated file There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's still referenced here There was a problem hiding this comment. Choose a reason for hiding this commentThe 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", | ||
|
@@ -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", | ||
|
@@ -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", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe 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", | ||
|
@@ -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" | ||
} | ||
} |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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...?