Support exactOptionalPropertyTypes for partial and optional helpers

[vlinder]

Hi!

Zod is adding the undefined type to all keys that are optional when using the different .partial() modifiers.

How about differentiating them by being present and undefined or just not present at all like typescript does with exactOptionalPropertyTypes: true.

Would it be possible to implement in zod? I guess it could be the default since if you don't care you don't care. Also just using the typescript Partial type will actually honor exactOptionalPropertyTypes by default (on the type level that is).

[akomm]

There was a discussion regarding this feature and it was silently closed. Regardless of people confirming its still a relevant issue for them (stale bot check ignored that fact).
They forcefully added | undefined to record, so that you can't use exactOptionalPropertyTypes: true properly anymore.

From what I'v observed so far, I doubt they will do anything. Now any Record that doesn't allow explicit undefined assignment with exactOptionalPropertyTypes: true still has to handle undefined case.

[scotttrinh]

Apologies for this being something that Stale bot closed, I absolutely think we should work with exactOptionalPropertyTypes. Unfortunately, it's a breaking change since our runtime code can't introspect your configuration at runtime, so it's not likely to be something that is fixed quickly.

Most likely, we'll introduce some new utility methods that have the new API in mind. PRs welcome!

[akomm]

Maybe there is some misunderstanding, but the change that was introduced to record is actually a breaking change.

Before the change to record, it was not possible to utilize the benefits of exactOptionalPropertyTypes. You could turn on the option, but there was no possibility to replicate {[P in Keys]?: Values} in zod. You could only replicate {[P in Keys]?: Values | undefined} or {[P in Keys]: Values}.

What the change to record does not provide is the replication of {[P in Keys]?: Values} which was missing before and it takes away the previously existing option of {[P in Keys]: Values} by forcing {[P in Keys]?: Values | undefined} on you.

I'm not sure here: do you consider type changes that suddenly lead to fail in a code that passed before not a breaking change? Do you only consider runtime fail a breaking change?

Because the record change was a breakage on type level. Whoever had previously exactOptionalPropertyTypes: false and did not check records key access, now fails type check (unless the usage-context accepted undefined as well). Even though the intention is correct, its still a breaking change.

The idea behind the option exactOptionalPropertyTypes was to keep old behavior when its off and to provide type correctness, when its on. Especially useful, when transforming between lookup (record) and list (array) and similar it matters.

Now you take away the intention of exactOptionalPropertyTypes: false by forcing it via | undefined but also deny the correctness gain from exactOptionalPropertyTypes: true.

[scotttrinh]

@akomm

I believe what you're describing (the change to make record values | undefined) is related, but not the same as the original post around partial (and therefore optional) if I'm reading it correctly. Hopefully we can fix this for the ZodRecord type as well, but I suspect it'll have a slightly different "fix" even if the motivation is the same.

I'm not sure here: do you consider type changes that suddenly lead to fail in a code that passed before not a breaking change? Do you only consider runtime fail a breaking change?

I absolutely consider both cases breaking changes, and we've accidentally broken code in the past, which is something I'd like to be more careful about in the future. There are exceptions to this: sometimes TypeScript versions can break our assumptions about how a feature works, and it can be hard to head off those changes in a flexible way. We'll do our best to react in those circumstances.

[akomm]

Its the same problem here, but with refinements. You're correct. Same points explained apply on those too.

[igalklebanov]

How about a new function (instead of reverting/fixing/introducing breaking changes)?

Maybe .questionable() ? get it? because it only adds ?s. 😏

[jameshfisher]

Moving my complaint in #1540 here, because it sounds like the same issue.

My specific problem is that I'm trying to parse values into this type:

type FormattedText = { text: string; bold?: true; };

The bold flag, if present, is always the value true. Example test cases:

// Valid values include
{ text: "foo" }
{ text: "foo", bold: true }

// Invalid values include
{ text: "foo", bold: false }
{ text: "foo", bold: undefined }

This schema is designed to work with Slate, and the values are in a database. The design is therefore non-negotiable.

I can't figure out how to configure zod to make the bold property optional, without also adding undefined to the type of the property. The .optional() method seems to do conflate the two: it makes the property optional, but also allows the value undefined if present. For example, zod.parse allows the value { text: "foo", bold: undefined }.

I realize I could do this with .refine(), but then the constraint is not represented in the type system. I want the type of .parse() to be { text: string; bold?: true; }.

[akomm]

@igalklebanov's proposal goes the right direction. Not sure about the name though.

It would have been perfect type-wise, if .optional() would leave the decision of allow undefined or not to the tsc and tsconfig.json. The problem is that the implementation of .optional() dosn't know at runtime what it should do. Therefore you need another API. What about: .exactOptional(), .optionalExact() or .optional({exact: true})?

This might also similarly apply to record constructor.

[vlinder]

I like the exactOptional name since it reflects the name of the tsconfig flag.

[zetaraku]

I think .optional() should not infer the undefined type just like how Optional Properties works in TypeScript,
and .partial() should simply make every property in the object optional just like how Partial<Type> works in TypeScript.

.optional() should align with the question mark ? in TypeScript.

If you really need to allow a property with an explicit undefined value, it should be explicitly stated using:

const personSchema = z.object({
  name: z.string().or(z.undefined())
});

type Person = z.infer<typeof personSchema>;

Oops, the Person type inferred is { name?: string | undefined; }.
It should be { name: string | undefined; } without a ?, since I don't even use .optional().

[akomm]

@zetaraku I think the problem is that while everything is fine type-wise when doing so, the code that runs at runtime need to know whether an explizit undefined value is acceptable. When exactOptional... is enabled, the expected runtime behavior for key?: number would be to fail parsing when key exists, but is undefined. But the runtime does not know under which TS settings the code was compiled. This is why it appears to me there is no way to signal this intention to the runtime besides via a separate refinement function that has this behavior AND typing.

[zetaraku]

@akomm

Since TypeScript actually differentiate { name?: string }, { name?: string | undefined }, and { name: string | undefined },
I think an undefined value should be forbidden by default unless explicitly allowed so that any of them can be simply composed by current .or(z.undefined()) and .optional() methods.

I would prefer zod to act as if exactOptionalPropertyTypes is true by default and provide an exact option (default to true) to change the behavior for both .optional() and .partial().

Usage like .optional({ exact: false }) or .partial({ exact: false }) may be rare,
but something like .optional({ exact: true }) or .partial({ exact: true }) everywhere will be a nightmare.

Reserving .optional() for true optional would be more rational.

[zetaraku]

A separate entry point like this might be a good non-breaking solution for both settings. 🤔

import { z } from 'zod/strict';

[zetaraku]

@akomm Since the only difference is the runtime behavior of .parse(), what if we just add a .parseExact() method instead and leave the type as is so that the inferred type can respect the exactOptionalPropertyTypes config and only users want to opt-in this stricter behavior have to use the method? This change is completely non-breaking and even fixes the previous breaking change.

[jameshfisher]

My main confusion here is why this is being treated as an "enhancement" and not a bug. Or worse, a regression: @akomm has indicated that this worked correctly in a previous version of zod.

[jameshfisher]

It seems that zod is designed around an assumed TypeScript config that its users are supposed to set. This makes sense: you can't design for all $2^{\text{flags}}$ TypeScript variants. Adding a new function or new import goes down a dangerous path, e.g. import ... from 'zod/exact_optional_properties_but_no_strict_null_checks_or_checked_index_access' .

But is it documented somewhere what TypeScript config zod is designed for, and why? As a design principle, I would design zod around the "strictest" form of TypeScript, i.e. for users that are using TypeScript effectively. So is there a particular reason that zod is designed for users with exactOptionalPropertyTypes turned off?

[akomm]

@jameshfisher 99% agree. It was designed for exactOptionalPropertyTypes: false. Now its designed for neither. That is the thing that baffled me. Instead of leave as-is or look for a solution to enable both, they just made something that is neither. This also breaks code within a feature version update.

I see where certain issues are with the feature, but there are solutions as discussed here. Probably this BC breakage would not have happened if author had not closed the topics that were discussing it, just to later add something that slices of quality, instead of adding to it or at least maintaining it.

I especially agree with the fact that adding namespaces feels like an overkill. However, because its not part of strict setting for a good reason, I wouldn't enforce it either, but simply provide the specific API that has defined behavior at runtime and accordingly the typing. Otherwise this feature could only come as a major release as changing the runtime behavior generally is BC break.

[akomm]

I want to add another example where this feature is useful but the change that was made to enforce | undefined falsifies the reality: Any json-based storage, be it arangodb, mongodb, file-system, ... JSON doesn't have undefined. When you parse such data, the typing now forces you to check undefined, while by design of t he storage format this case isn't possible.

[jameshfisher]

@akomm are you able to point us to the prior closed discussion on this topic, and/or the PR that changed the design of .optional() etc?

This thread is missing any input from people who want the current design. Or are there not any advocates for the current design? (And therefore another reason that we should just fix the bug?)

[akomm]

@jameshfisher there were a few, but one that has been reopened after the breakage is this:
#635

One thing I want to add though: Just because nobody comes up with a different expectation doesn't mean there are no. As long as the behavior is okay for them, they won't look for topics changing it. Until the breaking change. This should have triggered people, who did not explicitly check record keys. Their typing should yell now because of the forced | undefined change. The question is whether they will connect this with this topic...

[jameshfisher]

Aha - thanks @akomm. It looks like this issue #1510 is a duplicate of #635 - @vlinder would you agree?