fix(zod): handle content-type with charset suffix - #3433
Conversation
Match `application/json` and `multipart/form-data` content keys when they include parameters such as `; charset=utf-8`, and recognise vendor JSON subtypes (`application/*+json`, e.g. `application/geo+json`, `application/vnd.api+json`) so their response/request schemas are generated as well. Fixes orval-labs#3285 Co-authored-by: Pierre Isabel <205639300+pierre-isabel-bbc@users.noreply.github.com> Co-authored-by: melloware <4399574+melloware@users.noreply.github.com>
📝 WalkthroughWalkthrough
ChangesContent-type parsing and response schema generation
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Suggested labels
Suggested reviewers
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.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
This PR updates the Zod generator’s media-type detection to correctly handle charset parameters (and other ;... parameters) and recognize more JSON-like media types, then updates generated snapshots and adds a regression test.
Changes:
- Match JSON and multipart media types by normalizing away
; charset=...parameters and supportingapplication/*+jsonvariants. - Add a comprehensive test covering request/response content types with charset and multipart encoding behavior.
- Regenerate snapshots to reflect updated response schema generation (e.g.,
ListPetsResponseItem).
Reviewed changes
Copilot reviewed 22 out of 22 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/snapshots/zod/tags/pets.ts | Snapshot updated to include ListPetsResponseItem and array response schema. |
| tests/snapshots/zod/split/endpoints.ts | Snapshot updated for response item extraction and array response schema. |
| tests/snapshots/zod/schemas-false/endpoints.ts | Snapshot updated for response item extraction and array response schema. |
| tests/snapshots/zod/preprocess/preprocess.ts | Snapshot updated; response item schema now wrapped in zod.preprocess. |
| tests/snapshots/zod/petstore/endpoints.ts | Snapshot updated to include ListPetsResponseItem and ListPetsResponse. |
| tests/snapshots/zod/petstore-tags-split/pets/pets.ts | Snapshot updated to include response item extraction. |
| tests/snapshots/zod/branded-types/branded-types.ts | Snapshot updated; response array uses .brand<'ListPetsResponse'>(). |
| tests/snapshots/mcp/zod-schema-response/tool-schemas.zod.ts | Snapshot updated to include response item extraction and array response schema. |
| tests/snapshots/mcp/single/tool-schemas.zod.ts | Snapshot updated to include response item extraction and array response schema. |
| tests/snapshots/mcp/custom-server/tool-schemas.zod.ts | Snapshot updated to include response item extraction and array response schema. |
| tests/snapshots/hono/zod-schema-response/endpoints.zod.ts | Snapshot updated to include response item extraction and array response schema. |
| tests/snapshots/hono/petstore-tags/pets.zod.ts | Snapshot updated to include response item extraction and array response schema. |
| tests/snapshots/hono/petstore-tags-with-handlers/pets.zod.ts | Snapshot updated to include response item extraction and array response schema. |
| tests/snapshots/hono/petstore-tags-split/pets/pets.zod.ts | Snapshot updated to include response item extraction and array response schema. |
| tests/snapshots/hono/petstore-tags-split-with-handlers/pets/pets.zod.ts | Snapshot updated to include response item extraction and array response schema. |
| tests/snapshots/hono/petstore-split/endpoints.zod.ts | Snapshot updated to include response item extraction and array response schema. |
| tests/snapshots/hono/petstore-split-with-handlers/endpoints.zod.ts | Snapshot updated to include response item extraction and array response schema. |
| tests/snapshots/hono/petstore-single/endpoints.zod.ts | Snapshot updated to include response item extraction and array response schema. |
| tests/snapshots/hono/endpoint-parameters/endpoints.zod.ts | Snapshot updated to add generated response schemas and defaults for additional endpoints. |
| tests/snapshots/default/schemas-zod-only/endpoints.ts | Snapshot updated to include response item extraction and array response schema. |
| packages/zod/src/zod.test.ts | Adds regression test for charset-bearing media types and multipart encoding mapping. |
| packages/zod/src/index.ts | Implements media type normalization/matching and extracts helper for matching. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const isMediaType = | ||
| (pattern: string) => | ||
| ([contentType]: [string, object]): boolean => | ||
| new RegExp(pattern).test(contentType.split(';')[0].trim().toLowerCase()); | ||
|
|
| const isMediaType = | ||
| (pattern: string) => | ||
| ([contentType]: [string, object]): boolean => | ||
| new RegExp(pattern).test(contentType.split(';')[0].trim().toLowerCase()); |
| export const ListPetsResponseItem = zod | ||
| .union([ | ||
| zod | ||
| .union([ | ||
| zod.object({ | ||
| cuteness: zod.number(), | ||
| breed: zod.enum(['Labradoodle']), | ||
| }), | ||
| zod.object({ | ||
| length: zod.number(), | ||
| breed: zod.enum(['Dachshund']), | ||
| }), | ||
| ]) | ||
| .and( | ||
| zod.object({ | ||
| barksPerMinute: zod.number().optional(), | ||
| type: zod.enum(['dog']), | ||
| }), | ||
| ), | ||
| zod.object({ | ||
| petsRequested: zod.number().optional(), | ||
| type: zod.enum(['cat']), | ||
| }), | ||
| ]) | ||
| .and( | ||
| zod.object({ | ||
| '@id': zod.string().optional(), | ||
| id: zod.number(), | ||
| name: zod.string(), | ||
| tag: zod.string().optional(), | ||
| email: zod.string().email().optional(), | ||
| callingCode: zod.enum(['+33', '+420']).optional(), | ||
| country: zod.enum(["People's Republic of China", 'Uruguay']).optional(), | ||
| }), | ||
| ); |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/zod/src/zod.test.ts (1)
6279-6406: ⚡ Quick winAdd a focused
application/*+jsontest case.This new test locks charset handling, but it doesn’t explicitly assert vendor
+jsonmatching (for exampleapplication/vnd.api+json). Adding one small case here would directly protect the new regex behavior from regressions.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/zod/src/zod.test.ts` around lines 6279 - 6406, Add a small focused test that ensures vendor "+json" media types with charsets are matched by the new regex: call generateZod (same pattern used in the existing "content type with charset precision: comprehensive content type handling" test) with a schema whose response content key is "application/vnd.api+json; charset=utf-8" (or "application/ld+json; charset=utf-8") and assert the generated response Zod type (e.g., UploadFormResponse from the diff) is produced as a zod.object; this verifies generateZod's content-type matching logic handles application/*+json with charsets correctly.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@packages/zod/src/zod.test.ts`:
- Around line 6279-6406: Add a small focused test that ensures vendor "+json"
media types with charsets are matched by the new regex: call generateZod (same
pattern used in the existing "content type with charset precision: comprehensive
content type handling" test) with a schema whose response content key is
"application/vnd.api+json; charset=utf-8" (or "application/ld+json;
charset=utf-8") and assert the generated response Zod type (e.g.,
UploadFormResponse from the diff) is produced as a zod.object; this verifies
generateZod's content-type matching logic handles application/*+json with
charsets correctly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: b8c47035-7a20-4cf4-9382-f35a7ee66559
📒 Files selected for processing (22)
packages/zod/src/index.tspackages/zod/src/zod.test.tstests/__snapshots__/default/schemas-zod-only/endpoints.tstests/__snapshots__/hono/endpoint-parameters/endpoints.zod.tstests/__snapshots__/hono/petstore-single/endpoints.zod.tstests/__snapshots__/hono/petstore-split-with-handlers/endpoints.zod.tstests/__snapshots__/hono/petstore-split/endpoints.zod.tstests/__snapshots__/hono/petstore-tags-split-with-handlers/pets/pets.zod.tstests/__snapshots__/hono/petstore-tags-split/pets/pets.zod.tstests/__snapshots__/hono/petstore-tags-with-handlers/pets.zod.tstests/__snapshots__/hono/petstore-tags/pets.zod.tstests/__snapshots__/hono/zod-schema-response/endpoints.zod.tstests/__snapshots__/mcp/custom-server/tool-schemas.zod.tstests/__snapshots__/mcp/single/tool-schemas.zod.tstests/__snapshots__/mcp/zod-schema-response/tool-schemas.zod.tstests/__snapshots__/zod/branded-types/branded-types.tstests/__snapshots__/zod/petstore-tags-split/pets/pets.tstests/__snapshots__/zod/petstore/endpoints.tstests/__snapshots__/zod/preprocess/preprocess.tstests/__snapshots__/zod/schemas-false/endpoints.tstests/__snapshots__/zod/split/endpoints.tstests/__snapshots__/zod/tags/pets.ts
|
Thank you! |
|
Thanks for the fix! |
Summary
Fixes #3285.
packages/zod/src/index.tswas looking upapplication/jsonandmultipart/form-databy exact key, so:application/json; charset=utf-8(or any other media-type parameter) silently fell through and no Zod parser was generated, andapplication/geo+json,application/ld+json,application/vnd.api+jsonwere ignored even though they are valid JSON.This PR introduces a small
isMediaTypehelper that strips media-type parameters and matches with a regex (^application/([^/;]+\+)?json$for JSON,^multipart/form-data$for form data), so all of the above now produce the same Zod schemas as the bareapplication/jsoncase.Test Plan
packages/zod/src/zod.test.tscovering both shapes (multipart/form-data, parameterized JSON).tests/__snapshots__/.../*.zod.tsare updated — the same petstore spec exposes aListPetsResponseItem/ListPetsResponsethat previously wasn't generated because its response key carried a charset suffix.bun run format:check,bun run build,bun run typecheck,bun run lint,bun run test,bun run test:snapshots,bun run --filter orval-tests buildall green locally.Notes for reviewers
Supersedes #3404 and #3430 — both PRs implement the same
packages/zod/src/index.tschange. This PR takes the zod fix and the legitimatetests/__snapshots__/drift from those, and leavessamples/**untouched. (Thesamples/**undefined→void 0substitutions in #3430 were producing snapshot mismatches on CI because the orval mock generator only emitsundefined; see #3430 (comment) for the full diagnosis.)Full credit to @pierre-isabel-bbc for the original fix in #3404 and @melloware for picking it up in #3430 — both are tagged as co-authors on the commit.
Summary by CodeRabbit
New Features
application/*+json) and charset parameters in media types.Tests