feat(query): add literal param syntax for mutationInvalidates - #3237
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis PR adds support for literal parameter values in Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@tests/__snapshots__/fetch/mixed-content-responses-force-success/endpoints.ts`:
- Around line 86-93: The error-path unconditionally calls JSON.parse on body
which throws for non-JSON responses; wrap the parse in a try/catch inside the
res.ok false branch (where err is created) and set err.info to the parsed object
on success or to the raw body (or a safe text fallback like body || '') on
failure, preserving res.status in err.status; apply the same safe-parse pattern
to the other identical blocks that reference body, err, and res.status so
non-JSON payloads do not throw SyntaxError and mask the original HTTP error.
In `@tests/__snapshots__/fetch/mixed-content-responses/endpoints.ts`:
- Around line 44-47: The code unconditionally JSON.parse's any non-null body
when building getDataResponse, which will throw for the plain-text 429 payload;
update the getData construction (the body/data assignment logic around the body
variable and getDataResponse typing) to handle 429 specially — e.g., if
res.status === 429 set data to the plain string (or to the getDataResponse429
shape) instead of JSON.parse, or attempt JSON.parse in a try/catch and fall back
to the raw text for non-JSON 429 responses so getDataResponse and
getDataResponse429 are returned without runtime errors.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 909921a6-9a64-4c98-b98c-ef7f306f148a
📒 Files selected for processing (14)
docs/content/docs/reference/configuration/output.mdxpackages/core/src/types.tspackages/query/src/framework-adapter.tspackages/query/src/mutation-generator.tstests/__snapshots__/fetch/mixed-content-responses-force-success/endpoints.tstests/__snapshots__/fetch/mixed-content-responses-force-success/model/getData200.tstests/__snapshots__/fetch/mixed-content-responses-force-success/model/getMixedSuccess200.tstests/__snapshots__/fetch/mixed-content-responses-force-success/model/index.tstests/__snapshots__/fetch/mixed-content-responses/endpoints.tstests/__snapshots__/fetch/mixed-content-responses/model/getData200.tstests/__snapshots__/fetch/mixed-content-responses/model/getMixedSuccess200.tstests/__snapshots__/fetch/mixed-content-responses/model/index.tstests/__snapshots__/react-query/invalidates/endpoints.tstests/configs/react-query.config.ts
Allow `{ literal: "@me" }` in `params` to emit string literals instead
of variable references (`variables.@me`).
Closes orval-labs#3152
Signed-off-by: zeriong <jaeryong95@gmail.com>
25dcdda to
0099e13
Compare
There was a problem hiding this comment.
♻️ Duplicate comments (1)
tests/__snapshots__/fetch/mixed-content-responses-force-success/endpoints.ts (1)
86-93:⚠️ Potential issue | 🟠 MajorError parsing still assumes JSON in three non-JSON error paths.
Line 89, Line 132, and Line 183 unconditionally call
JSON.parse(...). If the server returns text/HTML, this throwsSyntaxErrorand masks the original HTTP failure.💡 Proposed fix (safe parse shared helper)
+const parseErrorInfo = (body: string | null, contentType?: string | null) => { + if (body === null) return {}; + const ct = (contentType ?? '').toLowerCase(); + if (!ct.includes('json')) return body; + try { + return JSON.parse(body); + } catch { + return body; + } +}; ... - const data = body !== null ? JSON.parse(body) : {}; + const data = parseErrorInfo(body, res.headers.get('content-type')); ... - const data = errorBody !== null ? JSON.parse(errorBody) : {}; + const data = parseErrorInfo(errorBody, res.headers.get('content-type')); ... - const data = body !== null ? JSON.parse(body) : {}; + const data = parseErrorInfo(body, res.headers.get('content-type'));#!/bin/bash # Verify all unconditional JSON.parse error-body sites in this snapshot file. rg -n --type=ts -C2 'JSON\.parse\((body|errorBody)\)' tests/__snapshots__/fetch/mixed-content-responses-force-success/endpoints.tsAlso applies to: 125-135, 180-187
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/__snapshots__/fetch/mixed-content-responses-force-success/endpoints.ts` around lines 86 - 93, The error handling unconditionally calls JSON.parse on body/errorBody (seen around the res.ok check creating err), which throws on non-JSON responses; update the three sites that parse response bodies (uses of JSON.parse(body) and JSON.parse(errorBody)) to safely parse: either check Content-Type for application/json before parsing or wrap JSON.parse in a try/catch and on failure set err.info to the raw body string (or an empty object) so the original HTTP error (err.status and message) is preserved; look for the res, body, err and errorBody variables and replace the direct JSON.parse calls with a safe-parse helper or inline try/catch to avoid throwing SyntaxError.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In
`@tests/__snapshots__/fetch/mixed-content-responses-force-success/endpoints.ts`:
- Around line 86-93: The error handling unconditionally calls JSON.parse on
body/errorBody (seen around the res.ok check creating err), which throws on
non-JSON responses; update the three sites that parse response bodies (uses of
JSON.parse(body) and JSON.parse(errorBody)) to safely parse: either check
Content-Type for application/json before parsing or wrap JSON.parse in a
try/catch and on failure set err.info to the raw body string (or an empty
object) so the original HTTP error (err.status and message) is preserved; look
for the res, body, err and errorBody variables and replace the direct JSON.parse
calls with a safe-parse helper or inline try/catch to avoid throwing
SyntaxError.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 9b563c85-614b-4d85-89fb-012219c03f91
📒 Files selected for processing (14)
docs/content/docs/reference/configuration/output.mdxpackages/core/src/types.tspackages/query/src/framework-adapter.tspackages/query/src/mutation-generator.tstests/__snapshots__/fetch/mixed-content-responses-force-success/endpoints.tstests/__snapshots__/fetch/mixed-content-responses-force-success/model/getData200.tstests/__snapshots__/fetch/mixed-content-responses-force-success/model/getMixedSuccess200.tstests/__snapshots__/fetch/mixed-content-responses-force-success/model/index.tstests/__snapshots__/fetch/mixed-content-responses/endpoints.tstests/__snapshots__/fetch/mixed-content-responses/model/getData200.tstests/__snapshots__/fetch/mixed-content-responses/model/getMixedSuccess200.tstests/__snapshots__/fetch/mixed-content-responses/model/index.tstests/__snapshots__/react-query/invalidates/endpoints.tstests/configs/react-query.config.ts
✅ Files skipped from review due to trivial changes (8)
- tests/snapshots/fetch/mixed-content-responses/model/getMixedSuccess200.ts
- tests/snapshots/react-query/invalidates/endpoints.ts
- tests/snapshots/fetch/mixed-content-responses-force-success/model/index.ts
- tests/snapshots/fetch/mixed-content-responses-force-success/model/getMixedSuccess200.ts
- tests/snapshots/fetch/mixed-content-responses/model/index.ts
- tests/snapshots/fetch/mixed-content-responses-force-success/model/getData200.ts
- tests/snapshots/fetch/mixed-content-responses/endpoints.ts
- tests/snapshots/fetch/mixed-content-responses/model/getData200.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- docs/content/docs/reference/configuration/output.mdx
- tests/configs/react-query.config.ts
- packages/query/src/mutation-generator.ts
- packages/core/src/types.ts
0099e13 to
101e54b
Compare
Summary
InvalidateTargetParamtype (string | { literal: string }) to@orval/core{ literal: "@me" }emits"@me"as a string literal in generated codevariables.<name>(backward-compatible)Usage
Test plan
Closes #3152
Summary by CodeRabbit
New Features
{ literal: 'value' }syntax alongside variable references inmutationInvalidatesconfiguration.Documentation