feat: unify direction param and add safe encode - #1
Open
skrushna-devzy wants to merge 1 commit into
Open
Conversation
Owner
Author
|
@devzyai review |
📖 WalkthroughThis change adds first-class “codec” support to Zod v4, introducing direction-aware encode/decode APIs (sync + async) alongside safe variants. A new encode-specific error type is added and re-exported for external consumers. Parsing exports gain encode/decode helpers and aliases with transform detection. JSON Schema generation is extended to understand codec types by referencing both input/output schemas. Tests are expanded to validate codec behavior, error paths, and assignability. 🔀 SequencesequenceDiagram
participant User
participant ZodSchema
participant Codec
participant Parser
User->>ZodSchema: encode/decode (or safeEncode/safeDecode)
ZodSchema->>Codec: Select direction (in/out) + run transforms
Codec->>Parser: parse/parseAsync with transform detection
Parser->>Codec: return value or issues
Codec->>User: value or ZodEncodeError / safe result
📂 File Changes📊 Changes by Category (5 categories)🔧 Codec support: direction-aware encode/decode API and schema typesIntroduces codec schema types and factories/constructors across core/classic/mini, enabling unified direction-aware encode/decode and safeEncode/safeDecode behavior.
🔧 Parsing API expansion: encoding helpers and decode aliasesAdds encode/encodeAsync/safeEncode/safeEncodeAsync helpers and decode/safeDecode (including async) aliases to the parsing API and exports across core/classic/mini.
🔧 Public error surface: introduce and export ZodEncodeErrorAdds a new core $ZodEncodeError and re-exports it as ZodEncodeError for external consumers in classic/mini entrypoints.
🔧 JSON Schema generation: codec-aware schema outputExtends JSON schema generation to understand codec types by processing in/out inner references and incorporating them into transforming checks.
🧪 Test coverage: codec behaviors, transform errors, and type-level expectationsAdds and updates tests validating codec encode/decode and safeEncode/safeDecode (including transform error cases), mini assignability/runtime behavior, and first-party type expectations for codec cases.
|
![devzyai-develop[bot]](https://avatars.githubusercontent.com/u/211546431?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review complete
DevzyAi finished this review for this commit. Feedback is in the inline review comments on this diff.
Files selected (15)
- packages/zod/src/v4/classic/external.ts (1)
- packages/zod/src/v4/classic/parse.ts (1)
- packages/zod/src/v4/classic/schemas.ts (3)
- packages/zod/src/v4/classic/tests/codec.test.ts (1)
- packages/zod/src/v4/classic/tests/firstparty.test.ts (4)
- packages/zod/src/v4/core/api.ts (1)
- packages/zod/src/v4/core/core.ts (1)
- packages/zod/src/v4/core/parse.ts (2)
- packages/zod/src/v4/core/schemas.ts (5)
- packages/zod/src/v4/core/to-json-schema.ts (2)
- packages/zod/src/v4/mini/external.ts (1)
- packages/zod/src/v4/mini/parse.ts (1)
- packages/zod/src/v4/mini/schemas.ts (1)
- packages/zod/src/v4/mini/tests/assignability.test.ts (1)
- packages/zod/src/v4/mini/tests/codec.test.ts (1)
Review comments generated (5)
- Review: 5
- LGTM: 0
Tips
Chat with DevzyAi Bot (@DevzyAi)
- Reply on review comments left by this bot to ask follow-up questions. A review comment is a comment on a diff or a file.
- Invite the bot into a review comment chain by tagging
@DevzyAiin a reply.
See More
Interact with @DevzyAi in any bot review thread (Files changed tab):
| Command | Description |
|---|---|
@DevzyAi explain |
Get a detailed explanation of the code or issue |
@DevzyAi fix |
Generate a code fix suggestion |
@DevzyAi suggest |
Get alternative implementations |
@DevzyAi ignore |
Mark this as a false positive |
@DevzyAi review |
Trigger a full PR review (overrides ignore) |
@DevzyAi test |
Generate unit tests for file(s) |
@DevzyAi help |
Show this help message |
Code suggestions
- The bot may make code suggestions, but please review them carefully before committing since the line number ranges may be misaligned.
- You can edit the comment made by the bot and manually tweak the suggestion if it is slightly off.
Pausing incremental reviews
- Add
@DevzyAi: ignoreanywhere in the PR description to pause further reviews from the bot.
| break; | ||
| default: | ||
| expectTypeOf(def).toEqualTypeOf<never>(); | ||
| expectTypeOf(def).toEqualTypeOf<never>(def); |
There was a problem hiding this comment.
🟠 High
expectTypeOf toEqualTypeOf called with wrong signature
💻 Syntax: toEqualTypeOf<never>(def) is an invalid usage and will fail typechecking.
expectTypeOf(def).toEqualTypeOf<never>(def); passes an argument to toEqualTypeOf that (per expect-typeof’s API) should not receive a value; the previous version correctly called it with no args. This will break the test build.
🛠️ Code Suggestions
- expectTypeOf(def).toEqualTypeOf<never>(def);
+ expectTypeOf(def).toEqualTypeOf<never>();🤖 AI Fix Prompt - Copy this into your AI coding agent
1. VALIDATION
Open packages/zod/src/v4/classic/tests/firstparty.test.ts and confirm the failing line is `expectTypeOf(def).toEqualTypeOf<never>(def);` and that expect-typeof’s `toEqualTypeOf` matcher is the zero-arg form (generic-only) in this repo’s typings. If the matcher is intentionally overloaded to accept a value here, don’t change it.
2. PROBLEM & LOCATION
File: packages/zod/src/v4/classic/tests/firstparty.test.ts
In the test that asserts a definition is `never`, the code calls `expectTypeOf(def).toEqualTypeOf<never>(def);`. Passing `def` as an argument is an invalid signature for `toEqualTypeOf` (it should not receive a runtime value), causing TypeScript typechecking to fail and breaking the test build.
3. FIX
Change the matcher call to use only the generic type parameter and no argument: `expectTypeOf(def).toEqualTypeOf<never>();`. Keep the rest of the assertion unchanged.
4. VERIFY
Run the TypeScript test/typecheck pipeline for this package (the same command used in CI for zod tests), and ensure packages/zod/src/v4/classic/tests/firstparty.test.ts compiles and the test suite still passes.
|
|
||
| default: | ||
| expectTypeOf(type).toEqualTypeOf<never>(); | ||
| expectTypeOf(type).toEqualTypeOf<never>(type); |
There was a problem hiding this comment.
🟠 High
expectTypeOf toEqualTypeOf called with wrong signature
💻 Syntax: toEqualTypeOf<never>(type) is an invalid usage and will fail typechecking.
Same issue as above: toEqualTypeOf should be invoked without a runtime argument; passing type will cause a TS error and fail CI.
🛠️ Code Suggestions
- expectTypeOf(type).toEqualTypeOf<never>(type);
+ expectTypeOf(type).toEqualTypeOf<never>();🤖 AI Fix Prompt - Copy this into your AI coding agent
1. VALIDATION
Open packages/zod/src/v4/classic/tests/firstparty.test.ts and find the expectTypeOf assertion that calls toEqualTypeOf with a generic and also passes a runtime argument. Confirm this is using vitest’s expectTypeOf (or equivalent) where toEqualTypeOf expects no value argument.
2. PROBLEM & LOCATION
In packages/zod/src/v4/classic/tests/firstparty.test.ts, in the test that asserts a value’s type is never, the code uses the pattern expectTypeOf(type).toEqualTypeOf<never>(type). This is the wrong signature: toEqualTypeOf is a type-only assertion and should not receive a runtime parameter; passing (type) causes TypeScript typechecking to fail and will break CI.
3. FIX
Change the call to remove the runtime argument so it becomes a pure type assertion:
Replace expectTypeOf(type).toEqualTypeOf<never>(type) with expectTypeOf(type).toEqualTypeOf<never>().
If there are other occurrences of toEqualTypeOf<...>(someValue) in this file, apply the same fix consistently.
4. VERIFY
Run the typecheck and the relevant test suite for this package (the command used in the repo for TS typechecking and the zod v4 classic tests). Ensure CI no longer fails on TypeScript errors in firstparty.test.ts.
Comment on lines
+8
to
+12
| function hasTransform(schema: schemas.$ZodType): boolean { | ||
| if (schema._zod.traits.has("$ZodTransform")) return true; | ||
| const def: any = schema._zod.def; | ||
| switch (def.type) { | ||
| case "pipe": |
There was a problem hiding this comment.
🟠 High
🐛 Bug: hasTransform() can infinite-recurse on lazy schemas that are self-referential, causing stack overflow during encode/safeEncode.
case "lazy" calls hasTransform(innerType) with no cycle detection. A common pattern is recursive schemas via z.lazy(() => schema), which will loop forever here and crash any encode path that calls hasTransform(schema).
🛠️ Code Suggestions
- function hasTransform(schema: schemas.$ZodType): boolean {
+ function hasTransform(schema: schemas.$ZodType, seen = new Set<any>()): boolean {
+ // Prevent infinite recursion for self-referential schemas (e.g. z.lazy(() => schema))
+ if (seen.has(schema)) return false;
+ seen.add(schema);
if (schema._zod.traits.has("$ZodTransform")) return true;
const def: any = schema._zod.def;
switch (def.type) {
case "pipe":
case "codec":
- return hasTransform(def.in) || hasTransform(def.out);
+ return hasTransform(def.in, seen) || hasTransform(def.out, seen);
case "array":
case "readonly":
case "optional":
case "nullable":
case "default":
case "prefault":
case "catch":
case "nonoptional":
- return hasTransform(def.innerType);
+ return hasTransform(def.innerType, seen);
case "object":
- return Object.values(def.shape).some((s: any) => hasTransform(s));
+ return Object.values(def.shape).some((s: any) => hasTransform(s, seen));
case "record":
- return hasTransform(def.valueType);
+ return hasTransform(def.valueType, seen);
case "map":
- return hasTransform(def.keyType) || hasTransform(def.valueType);
+ return hasTransform(def.keyType, seen) || hasTransform(def.valueType, seen);
case "set":
- return hasTransform(def.valueType);
+ return hasTransform(def.valueType, seen);
case "union":
- return def.options.some((s: any) => hasTransform(s));
+ return def.options.some((s: any) => hasTransform(s, seen));
case "tuple":
- return def.items.some((s: any) => hasTransform(s)) || (def.rest && hasTransform(def.rest));
+ return (
+ def.items.some((s: any) => hasTransform(s, seen)) ||
+ (def.rest && hasTransform(def.rest, seen))
+ );
case "lazy":
- return hasTransform((schema as any)._zod.innerType);
+ return hasTransform((schema as any)._zod.innerType, seen);
default:
return false;
}
}🤖 AI Fix Prompt - Copy this into your AI coding agent
1. VALIDATION
Open packages/zod/src/v4/core/parse.ts and find the hasTransform() helper used by encode/safeEncode. Confirm that for def.type === "lazy" it recurses into schema._zod.innerType without any cycle detection, and that a self-referential z.lazy(() => schema) would cause unbounded recursion.
2. PROBLEM & LOCATION
packages/zod/src/v4/core/parse.ts :: function hasTransform(schema)
The switch over def.type includes case "lazy" that does: hasTransform((schema as any)._zod.innerType) (and other cases recurse similarly) with no “seen” tracking. For recursive/lazy schemas that reference themselves (directly or indirectly), hasTransform() will infinite-recurse and stack overflow, crashing encode/safeEncode paths that call it.
3. FIX
Update hasTransform() to accept a “seen” Set (or ctx object) and short-circuit when a schema is revisited.
Implement it similarly to packages/zod/src/v4/core/to-json-schema.ts :: isTransforming, which uses ctx.seen to prevent cycles.
Concretely:
- Change signature to hasTransform(schema, seen = new Set()).
- At the top: if (seen.has(schema)) return false; then seen.add(schema).
- Thread the same seen Set through every recursive call (pipe/codec in+out, innerType wrappers, object shape values, union options, tuple items/rest, map key/value, record valueType, set valueType, and especially lazy).
- In the lazy case, recurse with hasTransform((schema as any)._zod.innerType, seen).
4. VERIFY
Run the zod package tests (or the repo test suite that covers encode/safeEncode).
Add/adjust a focused test that constructs a self-referential lazy schema (e.g., const A = z.lazy(() => A)) and calls encode/safeEncode (or whatever path triggers hasTransform) to ensure it no longer stack overflows and returns a stable result/error.
Comment on lines
+10
to
+14
| decode, | ||
| decodeAsync, | ||
| safeDecode, | ||
| safeDecodeAsync, | ||
| } from "../core/index.js"; |
There was a problem hiding this comment.
🟠 High
💻 Syntax: decode, decodeAsync, safeDecode, safeDecodeAsync may not be exported from ../core/index.js, causing a runtime ESM import error.
In ESM, re-exporting a non-existent named export throws at module load time (“does not provide an export named ...”), which would break any consumer importing packages/zod/src/v4/mini/parse.ts. Since this hunk only changes the re-export list, the fix is to ensure core actually exports these names or stop re-exporting them here.
🛠️ Code Suggestions
export {
parse,
safeParse,
parseAsync,
safeParseAsync,
encode,
encodeAsync,
safeEncode,
safeEncodeAsync,
- decode,
- decodeAsync,
- safeDecode,
- safeDecodeAsync,
} from "../core/index.js";🤖 AI Fix Prompt - Copy this into your AI coding agent
1. VALIDATION
Open packages/zod/src/v4/mini/parse.ts and packages/zod/src/v4/core/index.ts (or index.js source) and confirm whether ../core/index.js actually exports named exports decode, decodeAsync, safeDecode, safeDecodeAsync. If core already exports them (or mini/parse.ts isn’t part of the published ESM surface), skip the fix.
2. PROBLEM & LOCATION
packages/zod/src/v4/mini/parse.ts re-exports a list from "../core/index.js" and includes “decode, decodeAsync, safeDecode, safeDecodeAsync”. In ESM, “export { x } from '...';” throws at module load if the target module does not provide that named export, breaking any consumer that imports this mini entrypoint.
3. FIX
Make the re-export list match what core actually exports.
If core does NOT export these names: remove decode, decodeAsync, safeDecode, safeDecodeAsync from the export { ... } from "../core/index.js" list in packages/zod/src/v4/mini/parse.ts.
If core SHOULD export these names: add proper named exports in packages/zod/src/v4/core/index.ts by re-exporting them from the module that defines them (likely packages/zod/src/v4/core/parse.ts where decodeAsync/safeDecodeAsync exist), then keep the mini re-exports as-is. Ensure the compiled ../core/index.js ends up with real ESM named exports for all four symbols.
4. VERIFY
Rebuild the package output that produces packages/zod/src/v4/core/index.js and packages/zod/src/v4/mini/parse.ts, then run any module-load smoke test (Node ESM import) that imports the mini entrypoint. Also run the zod v4 parse/encode/decode related tests to ensure decode APIs are still reachable where expected.
Comment on lines
+1413
to
+1417
| ): ZodMiniCodec<A, B>; | ||
| export function codec(in_: SomeType, out: SomeType, handlers: any) { | ||
| return new ZodMiniCodec({ | ||
| type: "codec", | ||
| in: in_ as any as core.$ZodType, |
There was a problem hiding this comment.
🟠 High
🐛 Bug: codec() stores handlers.decode/encode without validation, so callers can pass missing/non-functions and trigger runtime TypeError during encode/decode.
Because handlers is any in the implementation signature (codec(in_: SomeType, out: SomeType, handlers: any)), TypeScript won’t reliably protect runtime usage (especially from JS consumers or as any). If handlers is malformed, the schema is constructed successfully but will crash later when core.$ZodCodec tries to call decode/encode, making failures late and harder to diagnose.
🛠️ Code Suggestions
export function codec(in_: SomeType, out: SomeType, handlers: any) {
+ if (!handlers || typeof handlers.decode !== "function" || typeof handlers.encode !== "function") {
+ throw new TypeError("codec() requires handlers: { decode: fn, encode: fn }");
+ }
return new ZodMiniCodec({
type: "codec",
in: in_ as any as core.$ZodType,
out: out as any as core.$ZodType,
decode: handlers.decode,
encode: handlers.encode,
}) as any;
}🤖 AI Fix Prompt - Copy this into your AI coding agent
1. VALIDATION
Open packages/zod/src/v4/mini/schemas.ts and find the exported function codec(in_, out, handlers). Confirm it forwards handlers.decode and handlers.encode into the ZodMiniCodec constructor without any runtime validation, and that handlers is typed as any (or otherwise not runtime-safe).
2. PROBLEM & LOCATION
File: packages/zod/src/v4/mini/schemas.ts
Region: export function codec(in_: SomeType, out: SomeType, handlers: any) { ... }
Problematic pattern: constructing new ZodMiniCodec with decode: handlers.decode and encode: handlers.encode while accepting handlers as any and not checking that decode/encode exist and are functions.
Why it matters: malformed handlers (missing decode/encode or non-functions) will let schema construction succeed but later crash at runtime with TypeError when encode/decode is invoked, producing late, hard-to-diagnose failures (especially for JS consumers or TS code using as any).
3. FIX
Add a runtime guard at the start of codec() that throws a TypeError if handlers is null/undefined or if handlers.decode/handlers.encode are not functions.
Keep the error message explicit (e.g., "codec() requires handlers: { decode: fn, encode: fn }") so callers immediately know what to pass.
Use packages/zod/src/v4/classic/schemas.ts :: codec as a behavioral reference for the expected shape (decode/encode functions), but implement the runtime check in the mini version before constructing ZodMiniCodec.
4. VERIFY
Run the zod package tests (or the repo test suite that covers v4/mini). Also grep for codec( usage in packages/zod/src/v4/mini to ensure no internal callers pass something other than { decode, encode }, and add/adjust a small unit test if there’s an existing mini schema test file that can assert codec() throws on invalid handlers.
|
@devzyai review |
|
🔍 Currently reviewing new changes in this PR. This may take a few minutes... Files selected (15)
|
|
👀 Review started Analyzing new changes in this PR. This may take a few minutes... |
2 similar comments
|
👀 Review started Analyzing new changes in this PR. This may take a few minutes... |
|
👀 Review started Analyzing new changes in this PR. This may take a few minutes... |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by DevzyAi