fix: superRefine() errors surfacing on optional() fields#5654
fix: superRefine() errors surfacing on optional() fields#5654tushargr0ver wants to merge 1 commit intocolinhacks:mainfrom
Conversation
There was a problem hiding this comment.
Medium priority — This PR addresses a legitimate regression from #5589. The approach of distinguishing optout = "optional" (key can be absent) from optout = "includeUndefined" (value type includes undefined, but validation runs) is sound.
However, the PR lacks a regression test for the bug it claims to fix. A test like the one from issue #5653 should be included to prevent future regressions:
test("superRefine on optional field in object", () => {
const schema = z.object({
field: z.any().optional().superRefine((val, ctx) => {
ctx.addIssue({ code: 'custom', message: 'should fail' });
})
});
// Should fail - superRefine adds an issue even for absent keys
expect(schema.safeParse({}).success).toEqual(false);
});Additionally, util.optionalKeys still checks optout === "optional", which will now only match exactOptional() fields. This appears to be dead code (computed but never used), but it should either be updated for consistency or removed.
| export interface $ZodExactOptionalInternals<T extends SomeType = $ZodType> | ||
| extends $ZodTypeInternals<core.output<T>, core.input<T>> { | ||
| def: $ZodExactOptionalDef<T>; | ||
| output: core.output<T>; // NO | undefined (narrowed from parent) | ||
| input: core.input<T>; // NO | undefined (narrowed from parent) | ||
| optin: "optional"; | ||
| optout: "optional"; | ||
| isst: never; | ||
| values: T["_zod"]["values"]; | ||
| pattern: T["_zod"]["pattern"]; | ||
| } |
There was a problem hiding this comment.
This interface now extends $ZodTypeInternals directly instead of $ZodOptionalInternals. This is necessary because $ZodExactOptional needs optout = "optional" while $ZodOptional now has optout = "includeUndefined". The approach makes sense, but the duplication of fields (optin, optout, isst, values, pattern) could be avoided by having both interfaces extend a shared base.
Not blocking, but consider whether there's a cleaner way to express this inheritance relationship.
cece1f3 to
9c56e85
Compare
|
Ultimately, this is behaving as intended. When you call .or(z.undefined) // | undefined
.exactOptional() // adds question mark, does not allow literal `undefined`These various kinds of optionality can be composed however you like to craft this logic. But if you want to add special refinements that only apply when a given key is present or not present on an object, that refinement needs to live at the object level. |
Bundled into colinhacks#4769 alongside the legitimate `optin = "optional"` fix for colinhacks#4768 without separate justification. The test added in the same commit literally has `// z.undefined should NOT be optional` directly above an assertion saying it is. `optout` is read by the object parser's absent-key error-swallow path (colinhacks#5589) and the tuple parser's `optStart`. Marking `z.undefined()` optout-optional conflates "value type is undefined" with "key may be absent in inferred output," which is upstream of the confusion in colinhacks#5654 and colinhacks#5661. Inference doesn't move: `\$ZodUndefinedInternals` doesn't promote `optin`/`optout` to required fields, so `z.object({ a: z.undefined() })` already infers as `{ a: undefined }` regardless. `optin = "optional"` stays — that one is the JSON-schema `required`-array fix from colinhacks#4768.
* fix(v4): apply trailing tuple defaults via optout flag
Tuple elements with `.default()` / `.prefault()` were silently dropped
when the input array was shorter than the tuple, because the parser
skipped any item past `input.length` whose slot was within `optStart`.
Gate the skip on `optout === "optional"` instead. That distinguishes
schemas that produce `undefined` for missing input (`.optional()`,
`z.undefined()`) — for which skipping is a no-op — from schemas that
produce a defined value. `optout` already bubbles through `nullable` /
`readonly` / `catch` / `pipe` / `union`, so chains like
`.default("x").nullable()` are covered without a type-name allowlist.
Closes #5229.
* test(v4): match tuple `_input` shape under tsc latest
`expectTypeOf<...>().toEqualTypeOf<[string, string?]>()` rejects the
inferred `[string, (string | undefined)?]` under TypeScript >=6 because
the optional element widens to `T | undefined` in the source position.
* fix(v4): keep tuple result dense when an optional precedes a default
The previous fix for #5229 left a sparse hole when an `.optional()`
slot sat between the input boundary and a later `.default()` — e.g.
`tuple([s, s.optional(), s.default("z")]).parse(["a"])` produced
`["a", <empty>, "z"]`, which fails `1 in r`, JSON-serializes to
`null`, and skips iteration.
Walk trailing items from the end to find the highest index whose
schema fills missing input (`optout !== "optional"`). Run every slot
up to that point so `.optional()` items reached while padding for a
later default produce explicit `undefined`. When the tail is purely
optional, the length-shortening behavior is preserved.
* refactor(v4): mirror $ZodObject in $ZodTuple parser
Drop the bespoke `runUntil` reverse-find scan in favor of the same
shape used by `$ZodObject`: run every item with `value: input[i]`
(undefined past the input boundary), let each schema decide what
undefined means, and have `handleTupleResult` swallow errors from
absent optional-out slots — the tuple-index analog of
`handlePropertyResult`'s `key in input` check.
A small post-loop trim drops trailing slots that produced `undefined`
for absent input (preserves the existing length-shortening behaviour
for purely-optional tails like `[s, s.optional()] / [a]`).
Behaviour identical to the previous fix on every case in the suite,
but the parser now reads alongside the object parser instead of
introducing a second, structurally-identical reverse-find boundary.
* fix(v4): break tuple parsing on first absent-optional rejection
When a tuple slot past `optStart` rejects `undefined` (e.g. an
`.optional()` chain with a refine that bans `undefined`), swallow the
issue and truncate the result there. Critically, also stop processing
later items so subsequent `.default()` slots do NOT materialize on top
of an already-malformed tail.
Previously the parser swallowed the absent-optional error but kept
running, letting a later default produce a value at an index past the
"missing" slot and yielding e.g. `["alpha", undefined, "d"]` for input
`["alpha"]`. Now the result is `["alpha"]`, matching the array-analog
of $ZodObject's absent-optional-key behaviour.
Implementation: parse all items in parallel, collect into an indexed
results array, then iterate in order during finalize — break on the
first absent-optional rejection, then run the trailing-undefined trim
so optional slots between the last real input and the rejected slot
also collapse away.
* test(v4): cover multi-trailing optional tuple cases
Lock in that:
- multiple trailing `.optional()` elements still trim back to the input
length (we don't fill the tail with literal `undefined`s)
- explicit `undefined` inside `input.length` IS preserved
- trailing optionals after a default that fires are still trimmed
* refactor(v4): hoist tuple finalize into top-level handleTupleResults
The post-processing for tuple parse results (in-order walk with
break-on-absent-optional-error, then trailing-undefined trim) was
defined as a closure inside the parse hot path. Hoist it to a
top-level `handleTupleResults` helper, mirroring the `handle*Result`
convention already used for objects and rest items.
Also document why `optStart` is intentionally NOT consulted in
finalize: it's an input-length concern handled by the `too_small`
precheck at the top of parse. Output shaping uses `optout` instead so
that a `.default()` tail item — which sits inside the optStart region
but materializes a defined value — is correctly preserved rather than
dropped or swallowed.
Adds a regression test that explicit `undefined` inside the input is
preserved even when the element schema produces `undefined` as a
valid output (e.g. `z.string().or(z.undefined())`,
`z.string().optional()`, `z.undefined()`). The trim's
\`i >= input.length\` floor is what guards this.
* fix: drop `z.undefined()`'s `optout = "optional"`
Bundled into #4769 alongside the legitimate `optin = "optional"` fix
for #4768 without separate justification. The test added in the same
commit literally has `// z.undefined should NOT be optional` directly
above an assertion saying it is.
`optout` is read by the object parser's absent-key error-swallow path
(#5589) and the tuple parser's `optStart`. Marking `z.undefined()`
optout-optional conflates "value type is undefined" with "key may be
absent in inferred output," which is upstream of the confusion in
#5654 and #5661.
Inference doesn't move: `\$ZodUndefinedInternals` doesn't promote
`optin`/`optout` to required fields, so `z.object({ a: z.undefined() })`
already infers as `{ a: undefined }` regardless. `optin = "optional"`
stays — that one is the JSON-schema `required`-array fix from #4768.
* fix: drop `z.undefined()`'s `optin = "optional"` too
Same conflation as the previous commit, on the input side. The motivating
JSON-schema bug from #4768 is `.catch()`-specific; the analogous extension
to `z.undefined()` was opportunistic. Under strict semantics
`z.object({ a: z.undefined() })` infers as `{ a: undefined }` (required
key) and the JSON-schema `required` array should agree. Runtime stays
permissive — `z.undefined().parse(undefined)` succeeds whether the source
was an absent key or an explicit `undefined`.
* test(v4): adapt tuple tests to z.undefined()'s required-input semantics
After dropping z.undefined()'s `optin`/`optout = "optional"` flags, a
trailing `z.undefined()` slot is required input — omitting it triggers
`too_small` rather than trimming. Update the assertions accordingly,
and lock in the error shape with inline snapshots since the precheck
abort behaviour (single `too_small`, no element-level errors) is the
exact piece worth pinning.
Also tighten the "required slot fails past input length" test to
snapshot the issue list instead of just `success === false`, so the
shape — single `too_small` from the precheck rather than e.g. an
`invalid_type` from the per-item run — is unambiguous.
* fix(v4): reject absent object keys when optin is required
Object parsing should only treat key absence as acceptable when the input-side optionality flag says so. Keep optout for output shaping, but stop letting schemas that merely accept or catch undefined make a required key disappear.
---------
Co-authored-by: Colin McDonnell <[email protected]>
Fixes #5653.
This PR updates ZodOptional to ensure validation chains (like superRefine) continue execution even if the value is undefined, correcting a regression where errors were swallowed.