-
-
Couldn't load subscription status.
- Fork 126
fix: zod4 compatibility with server package #2234
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
Conversation
|
Important Review skippedReview was skipped due to path filters ⛔ Files ignored due to path filters (3)
CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including You can disable this status message by setting the 📝 WalkthroughWalkthroughUpdates Zod handling and generated schemas for Zod v4 compatibility across multiple packages: improves lazy schema unwrapping, relaxes Decimal handling, makes record keys explicitly string-typed in many generated schemas, adjusts Zod error message extraction, tightens some tests, and bumps test scaffold/CLI Zod versions. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant C as Client
participant S as Server (REST / RPC)
participant Z as Zod
participant M as MessageUtils
C->>S: Submit payload
S->>Z: parse/validate(payload)
alt valid
Z-->>S: parsed data
S-->>C: 200 OK
else invalid
Z-->>S: ZodError<any>
S->>M: getZodErrorMessage(ZodError)
M-->>S: concise message
S-->>C: Error (message) + zodErrors
end
sequenceDiagram
autonumber
participant Caller as smartUnion / caller
participant Unwrap as unwrapLazy(schema)
Caller->>Unwrap: schema
alt not a ZodLazy
Unwrap-->>Caller: schema
else has unwrap()
Unwrap->>Unwrap: schema.unwrap()
Unwrap-->>Caller: unwrapped schema
else has .schema
Unwrap-->>Caller: schema.schema
else unknown shape
Unwrap-->>Caller: throw "Unable to determine how to unwrap a lazy schema..."
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Pre-merge checks (3 warnings)❌ Failed Checks (3 warnings)
✨ Finishing Touches🧪 Generate unit tests
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (2)
packages/runtime/src/enhancements/node/policy/handler.ts (1)
1265-1276: UseRecord<string, unknown>instead ofanyand drop the?? {}fallback
validateZodSchemaeither returns an object or throws, so the nullish-coalescing fallback is never needed.packages/server/src/api/rest/index.ts (1)
1972-2003: Widen ZodError type parameter — consistent with Zod v4Using ZodError avoids generic mismatches across Zod versions. Consider aligning internal casts to the same type for consistency.
Outside this hunk, in handlePrismaError:
// before err.meta?.zodErrors as ZodError // after (suggested) err.meta?.zodErrors as ZodError<any>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (3)
packages/runtime/package.jsonis excluded by!**/*.jsonpackages/server/package.jsonis excluded by!**/*.jsonpnpm-lock.yamlis excluded by!**/pnpm-lock.yaml,!**/*.yaml
📒 Files selected for processing (4)
packages/runtime/src/enhancements/node/policy/handler.ts(1 hunks)packages/runtime/src/zod-utils.ts(1 hunks)packages/server/src/api/rest/index.ts(2 hunks)packages/server/src/api/rpc/index.ts(3 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
packages/server/src/api/rpc/index.ts (1)
packages/runtime/src/local-helpers/zod-utils.ts (1)
getZodErrorMessage(8-22)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: build-test (20.x)
- GitHub Check: OSSAR-Scan
- GitHub Check: dependency-review
- GitHub Check: build-test (20.x)
- GitHub Check: build-test (20.x)
🔇 Additional comments (5)
packages/server/src/api/rest/index.ts (1)
203-211: z.record(z.string(), ...) for relationships — good Zod v4/v3 compatibilityExplicit string key schema is correct and future-proof. No runtime impact; validation gets stricter as intended.
packages/runtime/src/zod-utils.ts (1)
124-134: Robust lazy unwrapping across Zod versions — LGTMCovers both unwrap() and schema property; sensible error when unsupported.
packages/server/src/api/rpc/index.ts (3)
10-11: Centralize Zod error messages via getZodErrorMessage — LGTMRemoves direct dependency on zod-error helpers and unifies messaging.
204-218: makeError accepts ZodError — LGTMSignature matches wider Zod generics while preserving response shape.
254-256: Switch to getZodErrorMessage on validation failure — LGTMImproves compatibility across Zod versions and keeps concise messages.
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.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/runtime/src/enhancements/node/policy/handler.ts (1)
1260-1266: Null updates bypass Zod validation; includenullinliteralData
typeof null === 'object', so null assignments are excluded from validation, potentially letting invalidnullwrites slip through.Apply:
- const literalData = Object.entries(data).reduce<any>( - (acc, [k, v]) => ({ ...acc, ...(typeof v !== 'object' ? { [k]: v } : {}) }), - {} - ); + const literalData = Object.fromEntries( + Object.entries(data).filter(([, v]) => v === null || typeof v !== 'object') + );Add a test for updating a nullable and a non-nullable field with
null.packages/schema/src/plugins/zod/generator.ts (1)
401-405: Drop unused Decimal import from generated model files.
DecimalSchemano longer relies onDecimal; this import becomes unused and may trigger TS noUnusedLocals.if (decl.fields.some((field) => field.type.type === 'Decimal')) { writer.writeLine(`import { DecimalSchema } from '../common';`); - writer.writeLine(`import { Decimal } from 'decimal.js';`); }
🧹 Nitpick comments (4)
packages/schema/src/plugins/zod/transformer.ts (1)
489-490: Record key typing for JSON: LGTM; confirm top-level JSON null semantics
z.record(z.string(), jsonSchema.nullable())aligns with Zod v4. If top-level JSONnullshould be allowed (distinct from DB null), consider includingz.null()at the top-level union or ensure field-level.nullable()covers it.packages/server/src/api/rpc/index.ts (3)
169-171: Type consistency: cast to ZodError for clarity.Minor nit: keep the generic consistent with the updated signature.
- err.meta?.zodErrors as ZodError + err.meta?.zodErrors as ZodError<any>
97-104: Grammar nit in error text.Use “or” instead of uppercased “AND”.
- body: this.makeError('invalid request method, only PUT AND PATCH are supported'), + body: this.makeError('invalid request method, only PUT or PATCH are supported'),
194-199: Avoid leaking internal error details; use 500 for unknown errors.Returning raw
err.messagewith 400 can leak internals and misclassify server faults.- return { - status: 400, - body: this.makeError((err as Error).message), - }; + return { + status: 500, + body: this.makeError('internal server error'), + };Optionally add a correlation ID to logs/response meta if needed.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (9)
packages/plugins/trpc/tests/projects/nuxt-trpc-v10/package.jsonis excluded by!**/*.jsonpackages/plugins/trpc/tests/projects/nuxt-trpc-v11/package.jsonis excluded by!**/*.jsonpackages/plugins/trpc/tests/projects/t3-trpc-v11/package.jsonis excluded by!**/*.jsonpackages/runtime/package.jsonis excluded by!**/*.jsonpackages/server/package.jsonis excluded by!**/*.jsonpnpm-lock.yamlis excluded by!**/pnpm-lock.yaml,!**/*.yamltests/integration/test-run/package.jsonis excluded by!**/*.jsontests/integration/tests/frameworks/nextjs/test-project/package.jsonis excluded by!**/*.jsontests/integration/tests/frameworks/trpc/test-project/package.jsonis excluded by!**/*.json
📒 Files selected for processing (10)
packages/runtime/src/enhancements/node/policy/handler.ts(1 hunks)packages/runtime/src/zod-utils.ts(1 hunks)packages/schema/src/plugins/zod/generator.ts(2 hunks)packages/schema/src/plugins/zod/transformer.ts(1 hunks)packages/schema/src/plugins/zod/utils/schema-gen.ts(1 hunks)packages/server/src/api/rest/index.ts(2 hunks)packages/server/src/api/rpc/index.ts(3 hunks)script/test-scaffold.ts(1 hunks)tests/integration/tests/enhancements/with-password/with-password.test.ts(2 hunks)tests/integration/tests/enhancements/with-policy/field-validation.test.ts(16 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: build-test (20.x)
- GitHub Check: build-test (20.x)
- GitHub Check: build-test (20.x)
- GitHub Check: OSSAR-Scan
- GitHub Check: dependency-review
🔇 Additional comments (10)
tests/integration/tests/enhancements/with-password/with-password.test.ts (1)
97-98: Updated expectations to concise tokens: LGTMMatches the new
getZodErrorMessagestyle.Also applies to: 106-107
packages/schema/src/plugins/zod/utils/schema-gen.ts (1)
23-26: Zod v4-compatible record typing: LGTMSwitch to
z.record(z.string(), z.unknown())with preserved optionality is correct.packages/runtime/src/zod-utils.ts (1)
124-134: RobustunwrapLazyacross Zod versions: LGTMCovers
unwrap()and legacyschemaproperty with a clear error fallback.tests/integration/tests/enhancements/with-policy/field-validation.test.ts (1)
68-69: Assertion updates to tokenized messages: LGTMConsistent with shorter Zod error messaging and policy surfacing.
Also applies to: 101-102, 122-123, 169-178, 236-237, 249-250, 268-269, 286-287, 347-348, 360-361, 390-391, 406-407, 446-447, 462-463, 975-976
packages/server/src/api/rest/index.ts (2)
204-205: Use ofz.record(z.string(), ...)for relationships: LGTMCorrect for Zod v4 and keeps payload shape explicit.
1977-1978: Broadened type toZodError<any>: LGTMAvoids type incompatibility across Zod versions in error plumbing.
packages/server/src/api/rpc/index.ts (3)
10-10: Good swap to internal helper for Zod messages.Replacing the external
fromZodErrorwithgetZodErrorMessagekeeps dependencies lean and aligns with Zod v4 behavior.
204-206: makeError now accepts ZodError — aligned with Zod v4.Signature change is appropriate and non-breaking to callers here.
254-256: Validation message extraction updated correctly.Using
getZodErrorMessage(parseResult.error)produces concise messages while preservingzodErrorsin the payload.packages/schema/src/plugins/zod/generator.ts (1)
564-565: Explicit string-keyed records for atomic ops — good.
z.record(z.string(), z.unknown())matches Zod v4 expectations and removes implicit defaults.
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.
Actionable comments posted: 0
🧹 Nitpick comments (3)
packages/plugins/openapi/src/schema.ts (1)
24-25: DRY up repeated scopes schemaAll four occurrences are identical. Consider extracting a shared
ScopesSchemato reduce duplication and ease future changes.Apply:
import z from 'zod'; +const ScopesSchema = z.record(z.string(), z.string()); + export const SecuritySchemesSchema = z.record( z.string(), z.union([ @@ - scopes: z.record(z.string(), z.string()), + scopes: ScopesSchema, @@ - scopes: z.record(z.string(), z.string()), + scopes: ScopesSchema, @@ - scopes: z.record(z.string(), z.string()), + scopes: ScopesSchema, @@ - scopes: z.record(z.string(), z.string()), + scopes: ScopesSchema,Also applies to: 29-30, 34-35, 39-40
tests/integration/tests/cli/plugins.test.ts (1)
73-73: De-duplicate Zod version: pull from a shared constantReduce drift by centralizing the version in tests’ shared helpers.
Apply within this file:
- 'zod@^4.0.0', + 'zod@' + ZOD_VERSION,And add (outside this hunk):
- In tests/integration/tests/cli/share.ts:
export const ZOD_VERSION = '^4.0.0';
- Update imports at the top of this file:
import { ZOD_VERSION } from './share';tests/integration/tests/cli/generate.test.ts (1)
48-48: Reuse shared ZOD_VERSION for consistencySame suggestion as other suite to avoid future bumps in multiple places.
Apply within this file:
- installPackage('prisma @prisma/client zod@^4.0.0'); + installPackage('prisma @prisma/client zod@' + ZOD_VERSION);Also update the import (outside this hunk):
import { createNpmrc, ZOD_VERSION } from './share';
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml,!**/*.yamlpnpm-workspace.yamlis excluded by!**/*.yaml
📒 Files selected for processing (3)
packages/plugins/openapi/src/schema.ts(2 hunks)tests/integration/tests/cli/generate.test.ts(1 hunks)tests/integration/tests/cli/plugins.test.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
tests/integration/tests/cli/generate.test.ts (1)
packages/testtools/src/schema.ts (1)
installPackage(68-70)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: build-test (20.x)
- GitHub Check: OSSAR-Scan
- GitHub Check: build-test (20.x)
- GitHub Check: build-test (20.x)
- GitHub Check: dependency-review
🔇 Additional comments (4)
packages/plugins/openapi/src/schema.ts (1)
6-8: Good Zod v4 migration: explicit key schema for recordSpecifying
z.string()for the record keys aligns with Zod 4’s overloads and removes ambiguity. Looks correct.tests/integration/tests/cli/plugins.test.ts (2)
73-73: Zod v4 bump matches PR goalAligns test scaffold with repo-wide Zod v4 migration. Looks good.
73-73: Verify no lingering Zod v3 references
Double-check allpackage.jsonentries (dependencies, devDependencies, peerDependencies) and lockfiles for any^3.xZod pins to prevent mixed v3/v4 installs.tests/integration/tests/cli/generate.test.ts (1)
48-48: Upgrade to Zod v4 here is consistentKeeps this suite in sync with the v4 migration. Good.
No description provided.