fix(zod): add option to control zod version explicitly #2288
Conversation
📝 WalkthroughWalkthroughAdds configurable Zod versioning (accepts 'v3' or 'v4') by adding a zodVersion parameter, validating it in the generator, and threading it into the Transformer and generation paths so emitted imports and Zod type shapes reference the selected Changes
Sequence Diagram(s)sequenceDiagram
participant CLI as Generator
participant Gen as Zod generator
participant TF as Transformer
participant FS as Emitted files
CLI->>Gen: parse options (includes zodVersion)
Gen->>TF: construct Transformer(params + zodVersion)
TF->>TF: makeZodType(typeArg) -> versioned ZodType form
TF->>FS: emit imports "zod/{zodVersion}" and versioned schemas
note right of FS #D3E6FF: Outputs: schemas, inputs, barrel index, tests
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
⏰ 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)
🔇 Additional comments (1)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
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/schema/src/plugins/zod/transformer.ts (2)
41-56: DefaultzodVersionfor resilienceConsider defaulting to
'v3'ifparams.zodVersionis undefined to prevent accidental breakage.- this.zodVersion = params.zodVersion; + this.zodVersion = params.zodVersion ?? 'v3';
108-112: Avoidzod/${version}import here as wellSame module‑resolution risk as in generator. Prefer
import { z } from 'zod'(or@zenstackhq/runtime/zod).- let r = `import { z } from 'zod/${this.zodVersion}';\n`; + let r = `import { z } from 'zod';\n`;
♻️ Duplicate comments (3)
packages/schema/src/plugins/zod/generator.ts (1)
387-389: Same import-path concern as aboveThis repeats the
zod/${version}import risk. Please apply the same fix here.packages/schema/src/plugins/zod/transformer.ts (2)
505-507: SamemakeZodTypeissue on JSON schemaThe annotation
const jsonSchema: ${this.makeZodType('Prisma.InputJsonValue')}inherits the same problem. Will be fixed oncemakeZodTypeis corrected.
891-895: SamemakeZodTypeissue for operation argsThe mapped
Argstypes should bez.ZodType<Prisma.XxxArgs>; current v4 branch will not type‑check.
🧹 Nitpick comments (1)
packages/schema/src/plugins/zod/transformer.ts (1)
232-236: Optional: Preferz.iso.datetime()in v4 branchSmall polish: for DateTime, when targeting v4 you can emit
z.iso.datetime()instead ofz.string().datetime()to match new APIs; keep v3 as-is.Example:
- result.push(this.wrapWithZodValidators(['z.date()', 'z.string().datetime()'], field, inputType)); + result.push(this.wrapWithZodValidators( + this.zodVersion === 'v4' ? ['z.date()', 'z.iso.datetime()'] : ['z.date()', 'z.string().datetime()'], + field, inputType + ));Based on learnings
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
packages/schema/src/plugins/zod/generator.ts(8 hunks)packages/schema/src/plugins/zod/transformer.ts(8 hunks)packages/schema/src/plugins/zod/types.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
packages/schema/src/plugins/zod/transformer.ts (2)
packages/runtime/src/local-helpers/upper-case-first.ts (1)
upperCaseFirst(1-3)packages/sdk/src/utils.ts (1)
indentString(129-132)
packages/schema/src/plugins/zod/generator.ts (2)
packages/sdk/src/types.ts (1)
PluginError(115-119)packages/schema/src/plugins/zod/index.ts (1)
name(5-5)
⏰ 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: dependency-review
- GitHub Check: OSSAR-Scan
🔇 Additional comments (9)
packages/schema/src/plugins/zod/generator.ts (7)
53-54: LGTM: sane defaultDefaulting to
'v3'is fine.
165-166: ThreadingzodVersioninto Transformer looks goodPropagation is consistent.
266-267: Propagation LGTM
zodVersioncorrectly passed for enum generation.
297-298: Propagation LGTM
zodVersioncorrectly passed for object schemas.
733-738: Refine helper signature seems fine for both Zod v3 and v4Using
z.ZodType<T>is compatible. No issues spotted.Ensure downstream consumers aren’t relying on the old two‑generic form in their typings.
236-247: Ignore this review comment; the Zod subpath import pattern is valid and documented.Zod supports
zod/v3andzod/v4as public subpath exports that are intended to remain available permanently. The code at lines 236-247 generates imports likeimport { z } from 'zod/v3';(viazod/${this.zodVersion}), which is exactly the documented approach for version-specific code. This pattern will not fail module resolution—it's the recommended way to handle Zod's major version transitions.Likely an incorrect or invalid review comment.
79-87: Cannot definitively verify: PluginOptions is defined externallyThe
PluginOptionstype is imported from the external@zenstackhq/sdkpackage, so I cannot inspect its definition or verify whetherversionis documented as an official option key or whetherzodVersionis already part of it. Additionally:
- No documentation in this repository describes the expected
versionorzodVersionoption- No tests demonstrate the expected usage
- The review references a PR objective not visible in the codebase
The review's concern about naming clarity (using
zodVersionover genericversion) is reasonable, but without access to the external type definition and existing documentation/examples, I cannot confirm whether this is a breaking change, whether backward compatibility is needed, or if the suggested aliasing approach is compatible with howPluginOptionsis currently defined.packages/schema/src/plugins/zod/transformer.ts (2)
3-3: Minor import tidyMoving
upperCaseFirsthere is fine.
957-959: Verify the ZodType generic fix against actual Zod v4 type definitions and generated code behaviorBased on my research, there's conflicting evidence about the correctness of the current implementation:
Web search findings:
- v3 has three generics:
ZodType<Output, Def extends ZodTypeDef, Input>- v4 uses two generics:
ZodType<Output = unknown, Input = unknown>, with the Def generic removedCurrent code analysis:
- v3 generates:
z.ZodType<${typeArg}>(one generic)- v4 generates:
z.ZodType<${typeArg}, ${typeArg}>(two generics matching v4's Output and Input)Issue with the review's claim:
The review states dual generics "break v4," but the current v4 implementation appears semantically correct—it explicitly specifies both Output and Input as the same type. However, the review suggests a single generic works for both versions. I cannot find definitive evidence that the current v4 code causes type errors, nor can I confirm that single generic produces the intended behavior when Input defaults tounknownin v4.Recommendations for verification:
- Run TypeScript type checks against both generated schemas with Zod v3 and v4
- Confirm whether
z.ZodType<T>in v4 (with Input=unknown) is semantically acceptable for the generated code's use cases- Check if generated code that uses these type annotations actually compiles and behaves correctly
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
tests/regression/tests/issue-1378.test.ts (1)
3-46: Consider adding test coverage for Zod v4.Since the PR introduces configurable Zod versioning supporting both 'v3' and 'v4', consider adding a complementary test case to verify the v4 import path and generated code work correctly.
Add a new test case:
it('regression with zod v4', async () => { await loadSchema( ` model User { id String @id @default(cuid()) todos Todo[] } model Todo { id String @id @default(cuid()) name String @length(3,255) userId String @default(auth().id) user User @relation(fields: [userId], references: [id], onDelete: Cascade) @@allow("all", auth() == user) } `, { extraDependencies: ['zod'], zodVersion: 'v4', extraSourceFiles: [ { name: 'main.ts', content: ` import { z } from 'zod/v4'; import { PrismaClient } from '@prisma/client'; import { enhance } from '.zenstack/enhance'; import { TodoCreateSchema } from '.zenstack/zod/models'; const prisma = new PrismaClient(); const db = enhance(prisma); export const onSubmit = async (values: z.infer<typeof TodoCreateSchema>) => { await db.todo.create({ data: values, }); }; `, }, ], compile: true, } ); });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
tests/regression/tests/issue-1378.test.ts(1 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 (1)
tests/regression/tests/issue-1378.test.ts (1)
27-27: ****The
'zod/v3'import path is not an invalid reference to a non-existent npm package export. It's the expected generated code path from zenstack's zod plugin. WhenloadSchema()is called without an explicitzodVersionoption, it defaults to'v3', and the generated schemas in.zenstack/zod/will automatically use matching imports (import { z } from 'zod/v3'). The test file'sextraSourceFilescode correctly imports from the same generated path, so the imports will align. No explicit configuration or code changes are required.Likely an incorrect or invalid review comment.
|
fixes #2276 |
No description provided.