colinhacks / zod

TypeScript-first schema validation with static type inference
https://zod.dev
MIT License
33.92k stars 1.18k forks source link

.default() should apply default if wrapped schema transforms to undefined #3614

Open jedwards1211 opened 4 months ago

jedwards1211 commented 4 months ago

This may seem strange, but it's because I've been dealing with AWS CloudFormation parameters, where you can only have empty strings, not null or undefined values...

I was trying to do

// helper used in place of `.optional()` for my schema for parsing CloudFormation metadata
function optional<T extends z.ZodTypeAny>(schema: T) {
  return z
    .union([z.literal('').transform(() => undefined), schema.optional()])
    .optional()
}

const parameter = optional(z.string()).default('test')

I expected parameter.parse('') to be 'test' but instead it's undefined. This violates the output type of .default('test'), so I think even if the input is defined, ZodDefault should check that the parse output is too.

jedwards1211 commented 4 months ago

Here we're seeing the consequence of designing ZodDefault to change the input to a default value if it's undefined, rather than just changing the output value if it's undefined.

To guarantee that ZodDefault does produce the correct output type, I had to:

colinhacks commented 3 weeks ago

Here we're seeing the consequence of designing ZodDefault to change the input to a default value if it's undefined, rather than just changing the output value if it's undefined.

Indeed, ZodDefault is essentially a "short circuit" if the input is undefined. In my experience the majority of people expect it to work this way - operating as a "pre-check" instead of a "post-check". Though few people are every in a position to notice the difference.

The noUndefined that's used on the output type is also a decision that is more pragmatic than sound. A lot of people do z.string().optional().default() and expect the output type to be just string.

Perhaps Zod should just do both...short circuit with defaultValue if the input is undefined, otherwise run innerType.parse, check if the result if undefined, and return defaultValue if so. It feels insane but it might be the approach that behaves as expected for the greatest number of people (and it makes the output type with noUndefined 100% correct.

Let me know what you think, I'm evaluating changes to this behavior for Zod 4. cc @jedwards1211

jedwards1211 commented 3 weeks ago

Perhaps Zod should just do both...short circuit with defaultValue if the input is undefined, otherwise run innerType.parse, check if the result if undefined, and return defaultValue if so.

That's what I did in #3615. However, if a second default is applied, it's still impossible to get it to return the latter default if the initial input was not undefined. But I think it's an improvement nonetheless

jedwards1211 commented 3 weeks ago

In my experience the majority of people expect it to work this way - operating as a "pre-check" instead of a "post-check"

Was there a time when it didn't short-circuit, and then people complained that earlier nodes in the chain were running? Or was it just the fact that people asked for .default(a).default(b) to apply the latter default? (There would be ways to do that without short circuiting.)

Maybe Zod should add a .coalesce that does nullish coalescing in a non-short-circuit manner.