8000 optional should not add undefined to type signature · Issue #3186 · colinhacks/zod · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

optional should not add undefined to type signature #3186

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
hesxenon opened this issue Jan 24, 2024 · 24 comments
Open

optional should not add undefined to type signature #3186

hesxenon opened this issue Jan 24, 2024 · 24 comments
Labels
enhancement New feature or request

Comments

@hesxenon
Copy link

I'm sorry if this is a duplicate, I could not find an existing issue or discussion that seemed to be exactly about the following. It's really rather simple: the following schema infers the wrong type

const schema = z.object({
  foo: z.string().optional()
});

is inferred as

type Schema = {
  foo?: string | undefined
};

when it should really be

type Schema = {
  foo?: string
}

Let me showcase the issue with two different typescript playgrounds. They both have the same content, but the second has exactOptionalPropertyTypes enabled.

exactOptionalPropertyTypes: false

This playground errors, because even though we know that foo exists on result the compiler adds undefined to the set of possible types for result.foo.

exactOptionalPropertyTypes: true

And here we eliminated the error, because the compiler knows that if foo exists on result it must be exactly a string.

The problem to note here is that the decision to include undefined should be up to the compiler. Zod takes that decision away and subsequently makes it impossible to truly express optional keys.
@TahsinAyman
Copy link

ill try to fix this issue please

@JacobWeisenburger
Copy link
Contributor

image

@TahsinAyman
Copy link

thanks for explaining let me fix it
it would be appreciated if i were assigned in this task

@hesxenon
Copy link
Author

@JacobWeisenburger I'm not sure I follow, what's your point here?

It seems you've disabled exactOptionalPropertyTypes in your screenshot, hence the compiler adds undefined, but other than that I don't see what you're getting at :/

@JacobWeisenburger
Copy link
Contributor

Here's my tsconfig.json. It doesn't have exactOptionalPropertyTypes specified.

{
    "compilerOptions": {
        "lib": [
            "dom",
            "dom.iterable",
            "esnext"
        ],
        "module": "esnext",
        "target": "esnext",
        "moduleResolution": "bundler",
        "moduleDetection": "force",
        "allowImportingTsExtensions": true,
        "noEmit": true,
        "composite": true,
        "strict": true,
        "downlevelIteration": true,
        "skipLibCheck": true,
        "jsx": "preserve",
        "allowSyntheticDefaultImports": true,
        "forceConsistentCasingInFileNames": true,
        "allowJs": true,
        "types": [
            "bun-types"
        ]
    },
    "include": [
        "src/**/*",
        "package.json"
    ]
}

@JacobWeisenburger JacobWeisenburger added enhancement New feature or request and removed move-to-discussions? labels Jan 25, 2024
@hesxenon
Copy link
Author

yeah? The default is false; that doesn't make it any more correct for zod to take away the decision from the compiler for those that have it set to true.

Imho this isn't an enhancement but a bug - zod "deliberately" add's implicit nullability even if that nullability is explicitly forbidden. From what I'm reading from other issues in this repo I'm fairly certain that this is not what zod wanted or intended.

fyi: @types/bun exists by now, just in case.

@TahsinAyman
Copy link

i understood the problem and i also looked after the source code and understood what's wrong
let me try to fix it

@hesxenon
Copy link
Author
hesxenon commented Jan 29, 2024 via email

@TahsinAyman
Copy link

sorry im just a beginner.

@TahsinAyman TahsinAyman mentioned this issue Jan 29, 2024
Closed
@hesxenon
Copy link
Author
hesxenon commented Feb 3, 2024

the more I look at this, the more it looks like this won't be possible without a breaking change. While I did get correct optional inference working (basically just switch addQuestionMarks and baseObjectOutputType and infer Required Key with extends ZodOptional) the core of the problem is that ZodOptional adds |undefined to its in and output.

Removing that assumption breaks other tests because optional can't depend on its wrapping context.

E.g. here

const stringSchema = z.string().optional();

const objectSchema = z.object({
  foo: stringSchema
});

type S = z.infer<typeof stringSchema> // expectation: string | undefined
type O = z.infer<typeof objectSchema> // expectation: { foo?: string }

Since the stringschema can't know when it's being used in an object context it's impossible to correctly represent optional keys without also making the value "undefinedable".

Imho the only true solution (and maybe even the right one) would be to strictly reserve .optional() for usage within objects and to have a new operation like .nullable(), e.g. .orUndefined() for everything else.

@colinhacks , sorry for directly mentioning you but do you see any other solution?

@TahsinAyman
Copy link

🔥 I also tried but it didn't seem to make any sense to me 👎🏻

@TahsinAyman
Copy link

what i have come to notice when contributing is the ZodOptional is the one which adds | undefined on the T["output"]

class ZodOptional<T extends ZodTypeAny> extends ZodType<
  T["_output"] | undefined,
  ZodOptionalDef<T>,
  T["_input"] | undefined
>

if you remove the | undefined from here it seems to remove it but removes the ? from the key which dosent really make sense to me what so ever.

8000
@TahsinAyman
Copy link

@colinhacks nothing seems to be working. and is there any source code docs. and there is no comment / docs in the codebase!!! 👎🏻

@hesxenon
Copy link
Author
hesxenon commented Feb 23, 2024 •
edited
Loading

@TahsinAyman it's their prerogative not to consider this issue and they're under no obligation to write the codebase in any other way than it is now.

Honestly, if zod were my project I'd close this issue since

  1. considering the current structure and maturity of this project it just doesn't make sense to introduce a breaking change for an edge case (even though it's a valid case)
  2. fixing it without introducing a breaking change to the api would require an enormous rewrite

If people then started to complain about how my codebase doesn't cater to somebody elses taste I'd be less than willing to see their point.

For me it just meant sticking closer to FP patterns and using it like this:

z.object({
  foo: z.string().optional().transform(option.fromNullable)
})

@sp88011
Copy link
sp88011 commented Mar 1, 2024

@hesxenon can you expand on your workaround please? Where does fromNullable come from?

@hesxenon
Copy link
Author
hesxenon commented Mar 1, 2024

@sp88011 it comes from fp-ts' implementation of the option monad.

Since ZodObject makes each key optional where the value contains undefined (which is wrong) all you hav 8000 e to do is transform the value to something that does not include undefined after the optional codec.

@EphremDemlew
Copy link

I found a work arround in whitch you can make it a nullable too and when the type is infered the null is added as a type incase thats what you nedded

profileImage: z.string().optional().nullable(),

@fuadsaud
Copy link
fuadsaud commented Aug 21, 2024 •
edited
Loading

I have been bitten by this problem recently. Here's a simplified version of the use case I have:

type Entity = {
  name: string;
  description?: string;
};

const EntityCreationRequest = z.object({
  name: z.string(),
  description: z.string().optional(),
});

const entity: Entity = EntityCreationRequest.parse({
    name: "hi there",
});

The parse call above issues the following compiler error:

'entity' is declared but its value is never read.typescript(6133)
Type '{ name: string; description?: string | undefined; }' is not assignable to type 'Entity' with 'exactOptionalPropertyTypes: true'. Consider adding 'undefined' to the types of the target's properties.
  Types of property 'description' are incompatible.
    Type 'string | undefined' is not assignable to type 'string'.
      Type 'undefined' is not assignable to type 'string'.typescript(2375)

This is because the schema accepts a present, undefined description while my Entity type doesn't.

On one hand, Zod's notion of optionality belongs to the schema of the value itself, whereas the optionality in typescript is in the relationship between an object key and a value, so the current behavior doesn't seem completely wrong to me in this specific case, but it does feel wrong when applying partial or deepPartial to an object schema.

I guess the only non-breaking fix would be to introduce a new type of optionality, attached to the object key itself and have that behave differently, which would probably require changes to partial and deepPartial as well.

Unfortunately I'm not aware of a workaround at this moment.

@MaLiN2223
Copy link

I have been bitten by this problem recently. Here's a simplified version of the use case I have:

type Entity = {
  name: string;
  description?: string;
};

const EntityCreationRequest = z.object({
  name: z.string(),
  description: z.string().optional(),
});

const entity: Entity = EntityCreationRequest.parse({
    name: "hi there",
});

The parse call above issues the following compiler error:

'entity' is declared but its value is never read.typescript(6133)
Type '{ name: string; description?: string | undefined; }' is not assignable to type 'Entity' with 'exactOptionalPropertyTypes: true'. Consider adding 'undefined' to the types of the target's properties.
  Types of property 'description' are incompatible.
    Type 'string | undefined' is not assignable to type 'string'.
      Type 'undefined' is not assignable to type 'string'.typescript(2375)

This is because the schema accepts a present, undefined description while my Entity type doesn't.

On one hand, Zod's notion of optionality belongs to the schema of the value itself, whereas the optionality in typescript is in the relationship between an object key and a value, so the current behavior doesn't seem completely wrong to me in this specific case, but it does feel wrong when applying partial or deepPartial to an object schema.

I guess the only non-breaking fix would be to introduce a new type of optionality, attached to the object key itself and have that behave differently, which would probably require changes to partial and deepPartial as well.

Unfortunately I'm not aware of a workaround at this moment.

I encountered the same, did you manage to figure out how to get it fixed?

@acontreras89
Copy link

This is particularly annoying when using drizzle-zod, as things like:

const patchUserSchema = createInsertSchema(user).omit({ id: true }).partial()

will result in types that are incompatible with drizzle itself. (The same happens when extending with .optional())

Here's the workaround we had been using for some time (before disabling exactOptionalPropertyTypes altogether 🤣):

type RemoveUndefined<T> = {
  [K in keyof T]: Exclude<T[K], undefined>
}

export const removeUndefinedProps = <T extends object>(obj: T) => {
  return Object.entries(obj).reduce((acc, [key, value]) => {
    if (value !== undefined) acc[key as keyof T] = value
    return acc
  }, {} as RemoveUndefined<T>)
}

// ... later in the schema definition

const patchUserSchema = createInsertSchema(user)
  .omit({ id: true })
  .partial()
  .transform(removeUndefinedProps)

@darcyrush
Copy link
darcyrush commented Jan 16, 2025 •
edited
Loading

@TahsinAyman it's their prerogative not to consider this issue

This point is completely valid, but it would be nice to at least get the thoughts of one of the maintainers - although again, it is their prerogative to respond. Perhaps they are aware of a more generic workaround.

I tried @acontreras89 's solution but I'm not having much luck.

@TahsinAyman introduce a breaking change for an edge case (even though it's a valid case)

As for it being a breaking change, just thinking out loud, but would the impact of fixing this issue actually negatively impact most end users? I imagine the majority of users that are affected by this change are indeed using exactOptionalPropertyTypes: true so by fixing this issue (depending on how), I imagine that it would be more likely tooling and libraries impacted - which are generally more maintained.

@amyjko amyjko mentioned this issue Mar 22, 2025
Open
@bren-do
Copy link
bren-do commented Apr 24, 2025 •
edited
Loading

This issue is also affecting us. We are using a library that expects type signatures like:

type Schema = {
  foo?: string | null
};

And there is no way to infer that type from a Zod schema. The removeUndefinedProps workaround is a useful blunt instrument for removing all undefined from a type, but I can imagine situations where it would not be workable.

Perhaps, for a non-breaking way to resolve this issue, the z.object options could have a noImplicitUndefined flag that would not add undefined to the inferred type unless it is referenced explicitly.

@BrendanC23
Copy link

This will be implemented in Zod 4 (currently in beta). See the announcement.

@nahuel
Copy link
nahuel commented May 19, 2025

@BrendanC23 How you can get this definition using Zod 4?

type Schema = {
  foo?: string
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
0