Skip to content

feat: enhance fakeNonOptional to unwrap multiple inner optional layers and add test for nonoptional chain - #658

Open
InfiniteXyy wants to merge 1 commit into
soc221b:mainfrom
InfiniteXyy:main
Open

feat: enhance fakeNonOptional to unwrap multiple inner optional layers and add test for nonoptional chain#658
InfiniteXyy wants to merge 1 commit into
soc221b:mainfrom
InfiniteXyy:main

Conversation

@InfiniteXyy

@InfiniteXyy InfiniteXyy commented Jan 15, 2026

Copy link
Copy Markdown

Hi, first of all thanks for the great lib!

We encountered a problem while using it. We always extend a new schema based on an existing schema, but we run into issues when generating nonoptional.

Specifically, nonoptional does not work (if the schema itself is optional). I tried to fix this issue; could you help take a look?

Example

const schema = z.object({ name: z.string(), age: z.number().optional() })

const requiredSchema = schema.required()

fake(requiredSchema)

// expected: age alwasy be a number
// actual: age sometimes is undefined

My fix still has potential issues. if the schema looks like z.string().optional().nullable().nonoptional()

But this is quite uncommon, and I feel like it's not easy to solve this

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This PR updates the v4 Zod nonoptional faker behavior so fake() won’t accidentally generate undefined when nonoptional() is applied to a schema that still contains nested optional wrappers, and adds a regression test for an optional→nonoptional chain.

Changes:

  • Enhance fakeNonOptional to unwrap multiple nested $ZodOptional layers before faking the inner schema.
  • Add a nonoptional chain test to ensure z.string().optional().nonoptional() fakes to a string.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
tests/v4/zod.test.ts Adds a regression test for optional→nonoptional chaining.
src/v4/internals/schemas/non-optional.ts Unwraps repeated optional layers when faking nonoptional schemas.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread tests/v4/zod.test.ts
Comment on lines +818 to +820
test('nonoptional chain', { repeats: 5 }, () => {
const schema = z.string().optional().nonoptional()
const data = fake(schema)

Copilot AI Mar 7, 2026

Copy link

Choose a reason for hiding this comment

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

The new test relies on randomness (via faker's boolean in optional handling) to catch the regression, so it may still pass even if the bug is reintroduced (i.e., it can be flaky/ineffective as a regression test). Consider making it deterministic by seeding (seed(...)) or stubbing getFaker().datatype.boolean() for the duration of the test, and optionally asserting schema.parse(data) as well.

Suggested change
test('nonoptional chain', { repeats: 5 }, () => {
const schema = z.string().optional().nonoptional()
const data = fake(schema)
test('nonoptional chain', { repeats: 5 }, () => {
faker.seed(123)
const schema = z.string().optional().nonoptional()
const data = fake(schema)
schema.parse(data)

Copilot uses AI. Check for mistakes.
): Infer<T> {
return rootFake(schema._zod.def.innerType, context)
// Unwrap multiple inner optional layers if they exist
// A known issue is that this cannot handle `z.string().optional().nullable().nonoptional()`, But this should still solve most common cases

Copilot AI Mar 7, 2026

Copy link

Choose a reason for hiding this comment

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

The inline comment has a grammatical issue: the clause after the comma should not start with a capitalized “But”, and it would read clearer as a separate sentence. Please adjust the punctuation/capitalization so the comment is easier to read.

Suggested change
// A known issue is that this cannot handle `z.string().optional().nullable().nonoptional()`, But this should still solve most common cases
// A known issue is that this cannot handle `z.string().optional().nullable().nonoptional()`. This should still solve most common cases.

Copilot uses AI. Check for mistakes.
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.

2 participants