Skip to content

Conversation

@tushargr0ver
Copy link
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
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
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.

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

1 participant