Skip to content

fix: superRefine() errors surfacing on optional() fields#5654

Closed
tushargr0ver wants to merge 1 commit intocolinhacks:mainfrom
tushargr0ver:fix/optional-superrefine
Closed

fix: superRefine() errors surfacing on optional() fields#5654
tushargr0ver wants to merge 1 commit intocolinhacks:mainfrom
tushargr0ver:fix/optional-superrefine

Conversation

@tushargr0ver
Copy link
Copy Markdown
Contributor

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.

Copy link
Copy Markdown
Contributor

@pullfrog pullfrog Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Pullfrog  | Fix all ➔Fix 👍s ➔View workflow runpullfrog.com𝕏

Comment on lines +3378 to 3386
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"];
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@colinhacks
Copy link
Copy Markdown
Owner

Ultimately, this is behaving as intended. When you call .optional() on a schema, it means that omitting that field from any surrounding object is a valid state. That property bubbles up through the .superRefine() call. You can't add additional refinements on top of a absent value. There are other APIs for different types of optionality:

.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.

@colinhacks colinhacks closed this Apr 29, 2026
colinhacks added a commit to Cyjin-jani/zod that referenced this pull request Apr 29, 2026
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.
colinhacks added a commit that referenced this pull request Apr 29, 2026
* 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]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

superRefine() errors no longer surfaced if used in combination with optional() on an object() field that is not defined

2 participants