zod icon indicating copy to clipboard operation
zod copied to clipboard

.optional() and --exactOptionalPropertyTypes

Open bwbuchanan opened this issue 3 years ago • 30 comments

Currently, .optional() behaves like:

const user = z.object({
  username: z.string().optional(),
});
type C = z.infer<typeof C>; // { username?: string | undefined };

This results in a type mismatch when you're using --exactOptionalPropertyTypes in TypeScript 4.4 and expecting type C to match an interface definition like:

interface User {
  username?: string;
}

.partial(), .partialBy(), .deepPartial() all have the same issue.

It would be nice to unbundle the optionality of the key from the union type with undefined for the value.

I suggest that these methods be changed to specify the optional absence of the key by default, and perhaps accept an option to restore the old behavior of adding .or(z.undefined()) to the value schema(s). This would unfortunately be a breaking change, but it makes more sense than the current behavior, especially as more projects adopt --exactOptionalPropertyTypes.

bwbuchanan avatar Sep 06 '21 11:09 bwbuchanan

I soo want to start using exactOptionalPropertyTypes to put one more JavaScript The Bad Part(s)™️ behind. But our codebase uses Zod (❤️) heavily. Maybe I will take this on... @colinhacks Pull requests welcome? (As an opt-in feature until next major release?)

celmaun avatar Sep 10 '21 03:09 celmaun

I wonder if we can introduce this under a separate method like exactOptional? Or maybe have optional take a configuration parameter. If we go the config route, I'd want to get some direction from @colinhacks since there has been talks about using arguments to do things like custom error messages, etc, so we probably want to coordinate a bit.

scotttrinh avatar Sep 10 '21 17:09 scotttrinh

I don't think this there is any need for multiple methods for this, since if you don't have exactOptionalPropertyTypes enabled you can always assign undefined to an optional property, regardless of how it is typed.

Here is an example to just make all this explicit image If I disable the exactOptionalPropertyTypes TS option, then there are no errors. And of course, the bottom line is that we are missing an error for the line fooZod.a = undefined

ViktorQvarfordt avatar Oct 28 '21 13:10 ViktorQvarfordt

If we compare with io-ts, where everything seems to work with exactOptionalPropertyTypes, the behaviour is like this

To add undefined as part of a property, you use

t.type({
  v: t.union([t.undefined, t.string])
})

To make properties partial (i.e. with question mark), you use

t.partial({
  v: t.string
})

This leads to a very strict handling of undefined and question marks, which goes very well along with exactOptionalPropertyTypes.

It is important to note that when you use exactOptionalPropertyTypes you want good separation between undefined and question marks, so that if you want undefined, you get exactly undefined, and if you want question mark, you get exactly question mark - which it seems like io-ts is achieving, perhaps by accident, by using these somewhat more "primitive" constructs.

In Zod, we have .optional() which is backed by ZodOptional which you use like ZodOptional.create(z.string), and that basically adds undefined as part of the data type, so this is very similar to a construct like t.union([t.undefined, t.string]).

Then we have .partial() that you use on an object, and this actually just wraps the properties of the object in ZodOptional.

The addition of the question marks is done automatically by this code inside objectUtil that is employed by ZodObject to construct the output type

  type optionalKeys<T extends object> = {
    [k in keyof T]: undefined extends T[k] ? k : never;
  }[keyof T];

  type requiredKeys<T extends object> = Exclude<keyof T, optionalKeys<T>>;

  export type addQuestionMarks<T extends object> = {
    [k in optionalKeys<T>]?: T[k];
  } &
    { [k in requiredKeys<T>]: T[k] };

So the issue is that the question mark handling is currently inherently tied to whether the property type extends undefined, and in this case the question marks are added automatically.

ukarlsson avatar Oct 31 '21 17:10 ukarlsson

This might be drastically important when you work with document based databases, where a property might be a Dict of ID => VALUE

type ProductId = string & {readonly _: unique symbol}
type Entry = {id: ProductId; status: string}
type ProductsA = {[k in ProductId]: Entry}

declare const products: ProductsA
declare const productId: ProductId

// productA: Entry -> BAD, because products[productId] might be undefined
const productA = products[productId]

// Another approach, optional property
type ProductsB = {[k in ProductId]?: Entry}

declare const productsB: ProductsB

// product: Entry | undefined -> GOOD
const productB = productsB[productId]

// However, now we can:
productsB[productId] = undefined
// because without "exactOptionalPropertyTypes" enabled, {[k in ProductId]?: Entry} -> {[k in ProductId]?: Entry | undefined}

// also, given toArray conversion of a record:
type RecordValues<T> = T extends {[k: string | number | symbol]: infer V}
  ? V[]
  : never

// we get correctly, but unintentionally (Entry | undefined)[]
declare const arrayOfProductsB: RecordValues<ProductsB>

// "exactOptionalPropertyTypes" enabled behavior: don't append "undefined" to optional propertie's value type

akomm avatar Nov 04 '21 14:11 akomm

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

stale[bot] avatar Mar 02 '22 03:03 stale[bot]

I don't think this issue should be considered stale. It is a real issue that multiple users of the library are facing.

ViktorQvarfordt avatar Mar 03 '22 13:03 ViktorQvarfordt

Any progress on this ? Would you accept a pull request to add an exactOptional() for those that would want to use it without breaking existing code ?

vjau avatar Apr 20 '22 12:04 vjau

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

stale[bot] avatar Jun 19 '22 13:06 stale[bot]

Still relevant

akomm avatar Jun 19 '22 14:06 akomm

I don't think this should be closed due to staleness. This issue is currently blocking me from adopting --exactOptionalPropertyTypes and from the recent activity it looks like there are multiple users interested in a solution.

tstehr avatar Jun 26 '22 20:06 tstehr

Relevant issue just silently closed, regardless of activity, no word from author. Later can be understood if he has no time, but let bots close issues regardless of relevance and activity feels wrong.

akomm avatar Jul 12 '22 10:07 akomm

Another issue with this addQuestionMarks type – it looses the generic when use like this – the arbitrary field just becomes unexisted.

function dynamicZodObject<T>(t: T) {
  return z.object({
    arbitrary: z.any().refine((x): x is T => t),
  });
}

I spent so much time trying to fix it properly in zod to make a PR, but seems like it's just impossible. Right now i am fixing it like this

export type ZodObjectIdentity<T extends z.ZodRawShape, Side extends '_input' | '_output'> = {
  [K in keyof T]: T[K][Side];
};

export function zodObjectIdentity<T extends z.ZodRawShape>(
  t: T,
): z.ZodObject<T, 'strip', z.ZodTypeAny, ZodObjectIdentity<T, '_output'>, ZodObjectIdentity<T, '_input'>> {
  // @ts-ignore
  return z.object(t);
}

So, despite the zod is a great library, addQuestionMarks is the most bad design decision in zod. First of all – type: string | undefined is not the same as type?: string | undefined. Zod erases these boundaries, which is contradicts TypeScript-first schema validation library. And another issue – it looses generics, which is also a huge pitfall when we talk about Typescript-first In that case i would really prefer more-verbose io-ts decision with intersection([type(...), partial(...)])

dilame avatar Dec 05 '22 03:12 dilame

(Moving my complaints in https://github.com/colinhacks/zod/issues/1540 and https://github.com/colinhacks/zod/issues/1510 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 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 need the type of .parse() to be { text: string; bold?: true; }.

This issue is blocking me from using zod, because I can't find any reasonable workaround for it.

I understand 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?

jameshfisher avatar Jan 18 '23 14:01 jameshfisher

@jameshfisher I agree with most points here. I would like to agree on all, but there are maybe some conflicts I will explain further that we need to pay attention to. No separate namespaces: agree. Max possible strictness: agree. So where's the problem?

Well, exactOptionalPropertyTypes isn't part of strict for a reason. There is also the noUncheckedIndexedAccess option and others might come later. In the following example, we can see how the later partially takes away some benefits of the former.

See this in TS playground and toggle noUncheckedIndexedAccess.

// exactOptionalPropertyTypes always ON

type UserIdBrand = {readonly UserId: unique symbol}
type UserId = string & UserIdBrand

type User = {
  id: UserId
  name: string
}

declare const id: UserId
declare const r1: Record<UserId, User>
declare const r2: {[_ in UserId]: User}

// noUncheckedIndexedAccess "1" => u1 & u2: User | undefined (bad)
// noUncheckedIndexedAccess "0" => u1 & u2: User (good)
const u1 = r1[id]
const u2 = r2[id]

declare const a1: User[]
declare const t1: [User, User]

// noUncheckedIndexedAccess "1" => u3: User | undefined (good), u4: User (good)
// noUncheckedIndexedAccess "0" => u3: User (bad), u4: User (good)
const u3 = a1[0]
const u4 = t1[0]

What IndexAccess does could have been solved with exactOptional too, but it would have required an alternation to existing types, including Array, Record and more. I think the TS devs didn't want to introduce such a fundamental break in the entire core. The implication would require an entire separate discussion.

So we have this two features that didn't made it cleanly into the strict because of certain unsoundness left for the explained reasons, probably among others I can guess but let's not deviate off to much.

With exactOptional we have also some problems. The runtime behavior can not be enforced or guaranteed, even if you have it enabled. You might convert a Record<UserId, User> to Array<User> and Record<UserId, User | undefined> to Array<User | undefined>, but at runtime, you could still get an undefined value, but your code didn't test for this case. Now you will try to access properties on undefined when rendering, etc.

However, the changes made in zod have not improved, but worsen the situation as I've explained already.

This is why I think we need a specific API with type and runtime behavior guaranteed.

  1. optional() -> produces ?: T | undefined and will allow undefined at runtime
  2. optionalExact (or exactOptional or optional({exact: true})) -> produces ?: T and will fail parsing undefined value.

The 2. will however have the | undefined added when exactOptional is disabled.

akomm avatar Jan 26 '23 11:01 akomm

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

stale[bot] avatar Apr 26 '23 16:04 stale[bot]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

stale[bot] avatar Jul 25 '23 18:07 stale[bot]

Still relevant

luxaritas avatar Jul 25 '23 19:07 luxaritas

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

this worked for me: https://github.com/colinhacks/zod/discussions/2314#discussioncomment-5599193

EDIT: Nvm, after using the .parse() method it makes the validated data's properties back to type | undefined

yayza avatar Aug 14 '23 04:08 yayza

Still relevant

bazuka5801 avatar Aug 21 '23 13:08 bazuka5801

So many reactions! 🔥 So I wrote a workaround, check the PR at the link above.

This patch can be installed for testing by adding the following lines to package.json.

"zod": "npm:@bazuka5801/[email protected]"

⚠️ Be careful and careful, this patch may break your code.

bazuka5801 avatar Aug 22 '23 05:08 bazuka5801

@bazuka5801 thanks, this is very helpful! Would be great to see this approved & merged...

sweetpalma avatar Sep 21 '23 20:09 sweetpalma

Note this has actual runtime implications, e.g. 'a' in {} vs 'a' in { a: undefined }. Also Object.keys et al.

This is currently impacting me too.

Also something to consider (I'm not sure if this was brought up above) is whether...

z.object({ a: z.string().exactOptional() }).safeParse({ a: undefined })
z.object({ a: z.string() }).exactPartial().safeParse({ a: undefined })

...should fail to parse or instead parse successfully but omit the key (like JSON serialization behavior), i.e. a way to affect both Input and Output separately. I'm just thinking out loud here, but it might be nice if this was an option in exactOptional or maybe a different method so you could choose both behaviors (both of which could make sense in different scenarios?) Or can this behavior be simulated with current .optional()/.partial() + some transform?

EDIT: I think my last paragraph can be simulated like this.

type OmitUndefined<Object, Keys> = {
  [K in keyof Object]: K extends Keys
    ? Exclude<Object[K], undefined>
    : Object[K];
};

/**
 * Zod's `optional`, `partial`, etc. generate a type of `{ key?: SomeType | undefined }` but, since
 * we are using `exactOptionalPropertyTypes` in tsconfig, we want `key?: SomeType`.
 *
 * This function takes an object and removes the `undefined` from some optional properties.
 *
 * See https://github.com/colinhacks/zod/issues/635
 */
export function transformOmitPartial<
  Object extends object,
  Keys extends keyof Object,
>(keysToOmit: Array<Keys>): (value: Object) => OmitUndefined<Object, Keys>;
export function transformOmitPartial<Object extends object>(): (
  value: Object,
) => OmitUndefined<Object, keyof Object>;
export function transformOmitPartial<
  Object extends object,
  Keys extends keyof Object,
>(keysToOmit?: Array<Keys>): (value: Object) => OmitUndefined<Object, Keys> {
  return (value: Object) => {
    const realKeysToOmit =
      keysToOmit ?? (Object.keys(value) as Array<keyof Object>);

    for (const key of realKeysToOmit) {
      if (key in value && typeof value[key] === undefined) {
        delete value[key];
      }
    }

    return value as OmitUndefined<Object, Keys>;
  };
}

Not great because after .transform() you cannot .extend() for example, but it's... something.

alvaro-cuesta avatar Nov 22 '23 11:11 alvaro-cuesta

fwiw I would happily suffer through a major version bump to get rid of the optional key magic for any and unknown, as it's a non-trivial blocker to matching a pre-existing typescript type

dkrieger avatar Feb 14 '24 18:02 dkrieger

API-wise, rather than adding some new type, this should be behavior you can configure when initializing zod, e.g. {addQuestionMarks: false}. any, unknown, etc should not be made optional when that configuration is made. this will break exactly 0 existing use cases.

dkrieger avatar Feb 14 '24 18:02 dkrieger

i just want to call out that there is a footgun for consumers of zod types that needs to be avoided:

let's say i have a type for email recipients, consisting of the email address and an optional name.

interface EmailRecipient {
  email: string;
  name: string | undefined;
}

when users try to pass just an email, they are prompted by typescript to either pass a name or explicitly pass undefined. great.

when zod renders the type with a question mark,

interface EmailRecipient {
  email: string;
  name?: string | undefined;
}

a consumer can pass the email, typescript passes, and they have no idea that it's possible to pass a name. so there can be places littered around a codebase with inconsistent usage of the that name field. now the debugging developer has to trace every spot manually to figure out why the name isn't showing up.

this is exactly the sort of problem that typescript is supposed to help us avoid.

benhickson avatar May 15 '24 17:05 benhickson

@benhickson Just wanted to add that javascript CAN determine the difference at runtime, as it has been said by others, stated by @alvaro-cuesta i.e. And it definitely has runtime impact for libraries which treat them differently! A simple sample of code can demonstrate that:

const foo = {a: undefined};
// undefined
const bar = {};
// undefined
console.log(foo.a)
// undefined
console.log(bar.b)
// undefined
console.log(Object.hasOwn(foo, 'a'))
// true
console.log(Object.hasOwn(bar, 'a'))
// false

Then a logic as the following would be all wrong without the option to have exact optional properties (with runtime checks):

// obj defines properties to be set in database
function dbInsert(obj: Partial<Document>) {
  const documentInsert = {};
  for (const property in obj)
  {
    documentInsert[property] = obj[property];
  }
  database.insert(documentInsert);
  // error: field value cannot be undefined!
}

NorthBlue333 avatar Jun 04 '24 09:06 NorthBlue333

Thanks @NorthBlue333 , I've edited my comment. I think that makes it even more important to fix!

benhickson avatar Jun 04 '24 16:06 benhickson

Running into this exact issue using the drizzle-zod library. After a couple of wasted hours, discovered the cause...

We're approaching 3 (!) years since this issue was first brought up, does anyone have proposals for how to resolve this?

igniscyan avatar Jun 26 '24 17:06 igniscyan

For anyone not having checked out all the other links: According to https://github.com/colinhacks/zod/pull/2675 this is on the roadmap for ZOD 4 and a RFC will be published "soon".

eikowagenknecht avatar Jun 28 '24 10:06 eikowagenknecht