-
-
Couldn't load subscription status.
- Fork 246
Integrate graphql-codegen to generate GraphQL types #2155
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
Changes from 17 commits
5a21617
8402bf6
9a3a5e8
441e25c
5c8b6e1
55f86cc
0e7055d
dd5d2fe
1fc0818
09b8ca5
a239467
d71e394
7bb3bc6
f15fe68
e17d036
cb90877
bcf8d26
4babd73
f1c7653
3316cce
6bb7ed0
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 |
|---|---|---|
| @@ -1,2 +1,3 @@ | ||
| .pnpm-store/ | ||
| pnpm-lock.yaml | ||
| src/types/__generated__/**/* |
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,62 @@ | ||||||||||||||||||||||||||||||||||||||||||
| import { CodegenConfig } from '@graphql-codegen/cli' | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| const PUBLIC_API_URL = process.env.PUBLIC_API_URL || 'http://localhost:8000' | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
rudransh-shrivastava marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||||||||||||||||||||||||||||||
| export default (async (): Promise<CodegenConfig> => { | ||||||||||||||||||||||||||||||||||||||||||
| const response = await fetch(`${PUBLIC_API_URL}/csrf/`, { | ||||||||||||||||||||||||||||||||||||||||||
| method: 'GET', | ||||||||||||||||||||||||||||||||||||||||||
| }) | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| if (!response.ok) { | ||||||||||||||||||||||||||||||||||||||||||
| throw new Error(`Failed to fetch CSRF token: ${response.status} ${response.statusText}`) | ||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||
| const data = await response.json() | ||||||||||||||||||||||||||||||||||||||||||
| const csrfToken = data.csrftoken | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
|
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. 🛠️ Refactor suggestion Harden CSRF extraction and clear timeout. Be resilient to different payload shapes and fall back to Set-Cookie. Fail fast if still missing. - if (!response.ok) {
+ clearTimeout(timeout)
+ if (!response.ok) {
throw new Error(`Failed to fetch CSRF token: ${response.status} ${response.statusText}`)
}
- const data = await response.json()
- const csrfToken = data.csrftoken
+ type CsrfResponse = { csrftoken?: string; csrfToken?: string; token?: string }
+ const data = (await response.json().catch(() => ({}))) as CsrfResponse
+ let csrfToken = data.csrftoken ?? data.csrfToken ?? data.token
+ if (!csrfToken) {
+ const setCookie = response.headers.get('set-cookie') ?? ''
+ const m = setCookie.match(/(?:^|;\s*)csrftoken=([^;]+)/i)
+ if (m) csrfToken = m[1]
+ }
+ if (!csrfToken) {
+ throw new Error('Failed to extract CSRF token from CSRF endpoint response')
+ }📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||
| return { | ||||||||||||||||||||||||||||||||||||||||||
| documents: ['src/**/*.{ts,tsx}', '!src/types/__generated__/**'], | ||||||||||||||||||||||||||||||||||||||||||
| generates: { | ||||||||||||||||||||||||||||||||||||||||||
| './src/': { | ||||||||||||||||||||||||||||||||||||||||||
| config: { | ||||||||||||||||||||||||||||||||||||||||||
| avoidOptionals: { | ||||||||||||||||||||||||||||||||||||||||||
| // Use `null` for nullable fields instead of optionals | ||||||||||||||||||||||||||||||||||||||||||
| field: true, | ||||||||||||||||||||||||||||||||||||||||||
| // Allow nullable input fields to remain unspecified | ||||||||||||||||||||||||||||||||||||||||||
| inputValue: false, | ||||||||||||||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||||||||||||||
| // Use `unknown` instead of `any` for unconfigured scalars | ||||||||||||||||||||||||||||||||||||||||||
| defaultScalarType: 'unknown', | ||||||||||||||||||||||||||||||||||||||||||
| // Apollo Client always includes `__typename` fields | ||||||||||||||||||||||||||||||||||||||||||
| nonOptionalTypename: true, | ||||||||||||||||||||||||||||||||||||||||||
| // Apollo Client doesn't add the `__typename` field to root types so | ||||||||||||||||||||||||||||||||||||||||||
| // don't generate a type for the `__typename` for root operation types. | ||||||||||||||||||||||||||||||||||||||||||
| skipTypeNameForRoot: true, | ||||||||||||||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||||||||||||||
| // Order of plugins matter | ||||||||||||||||||||||||||||||||||||||||||
| plugins: ['typescript-operations', 'typed-document-node'], | ||||||||||||||||||||||||||||||||||||||||||
| preset: 'near-operation-file', | ||||||||||||||||||||||||||||||||||||||||||
| presetConfig: { | ||||||||||||||||||||||||||||||||||||||||||
| // This should be the file generated by the "typescript" plugin above, | ||||||||||||||||||||||||||||||||||||||||||
| // relative to the directory specified for this configuration | ||||||||||||||||||||||||||||||||||||||||||
| baseTypesPath: './types/__generated__/graphql.ts', | ||||||||||||||||||||||||||||||||||||||||||
| // Relative to the source files | ||||||||||||||||||||||||||||||||||||||||||
| folder: '../../types/__generated__', | ||||||||||||||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||||||||||||||
| './src/types/__generated__/graphql.ts': { | ||||||||||||||||||||||||||||||||||||||||||
| plugins: ['typescript'], | ||||||||||||||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||||||||||||||
| // Don't exit with non-zero status when there are no documents | ||||||||||||||||||||||||||||||||||||||||||
| ignoreNoDocuments: true, | ||||||||||||||||||||||||||||||||||||||||||
| overwrite: true, | ||||||||||||||||||||||||||||||||||||||||||
| schema: { | ||||||||||||||||||||||||||||||||||||||||||
| [`${PUBLIC_API_URL}/graphql/`]: { | ||||||||||||||||||||||||||||||||||||||||||
| headers: { | ||||||||||||||||||||||||||||||||||||||||||
| Cookie: `csrftoken=${csrfToken}`, | ||||||||||||||||||||||||||||||||||||||||||
| 'X-CSRFToken': csrfToken, | ||||||||||||||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||
| })() | ||||||||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -8,6 +8,7 @@ | |||||||||||||||||
| "dev": "next dev --port 3000", | ||||||||||||||||||
| "format": "prettier --log-level warn --write .", | ||||||||||||||||||
| "format:check": "prettier --check .", | ||||||||||||||||||
| "graphql-codegen": "graphql-codegen --config graphql-codegen.ts", | ||||||||||||||||||
|
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. 🛠️ Refactor suggestion Generate types automatically during builds. Prevent stale/missing types by invoking codegen before build. "scripts": {
- "build": "next build",
+ "build": "pnpm run graphql-codegen && next build",
"build:turbo": "next build --turbo",
@@
- "graphql-codegen": "graphql-codegen --config graphql-codegen.ts",
+ "graphql-codegen": "graphql-codegen --config graphql-codegen.ts",
+ "predev": "pnpm run graphql-codegen",
+ "pretest:unit": "pnpm run graphql-codegen",📝 Committable suggestion
Suggested change
🤖 Prompt for AI AgentsThere 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. how is this possible if we run backend separately? if we invoke codegen before build it may fail as backend may or may not be available. Codegen requires a server to query the GraphQL endpoint 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. @rudransh-shrivastava You're absolutely right! That's a critical oversight on my part. Running codegen during build would indeed fail when the backend isn't available, which is common in CI/CD pipelines or local development scenarios. Here are better approaches to handle this: Option 1: Use a static schema file // In graphql-codegen.ts, replace the schema URL with a local file
- schema: `${PUBLIC_API_URL}/graphql/`,
+ schema: './schema.graphql',Option 2: Make codegen conditional {
"scripts": {
"graphql-codegen": "graphql-codegen --config graphql-codegen.ts",
"graphql-codegen:safe": "graphql-codegen --config graphql-codegen.ts || echo 'Codegen skipped - server unavailable'",
"build": "pnpm run graphql-codegen:safe && next build"
}
}Option 3: Separate CI step Given that you've already committed the generated types, Option 3 seems to align with your current approach. The types would be generated when the backend is available and committed to the repo. What's your preference for handling this in your deployment pipeline? 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 would like to know if there's a way to run graphql-codegen after backend is available? This is for local development purposes. I am assuming that for CI jobs -- all generated files are already present. 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 first solution with |
||||||||||||||||||
| "lighthouse-ci": "lhci autorun", | ||||||||||||||||||
| "lighthouse-ci:desktop": "lhci autorun --collect.settings.preset=desktop", | ||||||||||||||||||
| "lint": "eslint . --config eslint.config.mjs --fix --max-warnings=0", | ||||||||||||||||||
|
|
@@ -23,6 +24,7 @@ | |||||||||||||||||
| "@fortawesome/free-regular-svg-icons": "^6.7.2", | ||||||||||||||||||
| "@fortawesome/free-solid-svg-icons": "^6.7.2", | ||||||||||||||||||
| "@fortawesome/react-fontawesome": "^3.0.1", | ||||||||||||||||||
| "@graphql-typed-document-node/core": "^3.2.0", | ||||||||||||||||||
|
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. Does apollo pick up the generated types automagically based on the location? 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. No, We need to import the types in each file where Is it okay if I refactor all this in a different PR? I will need to update the tests too. I believe this refactor should complete the apollo migration. For now, the generated files are unused. It's recommended by apollo to separate out queries (one generated file for each query/mutation file) for larger applications. If we don't do this, all the queries and mutation are generated in the |
||||||||||||||||||
| "@heroui/button": "^2.2.24", | ||||||||||||||||||
| "@heroui/modal": "^2.2.21", | ||||||||||||||||||
| "@heroui/react": "^2.8.2", | ||||||||||||||||||
|
|
@@ -68,6 +70,11 @@ | |||||||||||||||||
| "@axe-core/react": "^4.10.2", | ||||||||||||||||||
| "@eslint/eslintrc": "^3.3.1", | ||||||||||||||||||
| "@eslint/js": "^9.34.0", | ||||||||||||||||||
| "@graphql-codegen/cli": "^5.0.7", | ||||||||||||||||||
| "@graphql-codegen/near-operation-file-preset": "^3.1.0", | ||||||||||||||||||
| "@graphql-codegen/typed-document-node": "^5.1.2", | ||||||||||||||||||
| "@graphql-codegen/typescript": "^4.1.6", | ||||||||||||||||||
| "@graphql-codegen/typescript-operations": "^4.6.1", | ||||||||||||||||||
| "@lhci/cli": "^0.15.1", | ||||||||||||||||||
| "@playwright/test": "^1.55.0", | ||||||||||||||||||
| "@swc/core": "^1.13.5", | ||||||||||||||||||
|
|
||||||||||||||||||
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.
This should probably be part of
make checktarget if you skip it here.Also this way we'll avoid adding local pre-commit hook
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.
I've made this change. Thank you