Skip to content
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.

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

Record types should be partial #751

Closed
jeremybull opened this issue Nov 6, 2021 · 8 comments · Fixed by #752
Closed

Record types should be partial #751

jeremybull opened this issue Nov 6, 2021 · 8 comments · Fixed by #752
Labels
enhancement New feature or request

Comments

@jeremybull
Copy link
Contributor

jeremybull commented Nov 6, 2021

I love that in v3.9 we can now specify the type of the record key using enums or literals.

But the inferred Record type now needs to be wrapped in Partial, otherwise typescript forces us to specify every key, which is inconsistent with how the Zod parser interprets the same schema.

For example, suppose we create a schema that allows us to map states to temperatures:

const Weather = z.object({
    temperatures: z.record(z.enum(['VT', 'NH', 'CA', 'AZ']), z.enum(['warm', 'cold']))
});

If we map a couple of states, it parses fine. This parses without throwing:

Weather.parse({ temperatures: {VT: 'cold', CA: 'warm'} });

But if we try to use the inferred type with the same object, typescript is not happy:

type Weather = z.infer<typeof Weather>;
const weather: Weather = { temperatures: {VT: 'cold', CA: 'warm'} };

This gives us this error:

Type '{ VT: "cold"; CA: "warm"; }' is missing the following properties from type
'Record<"VT" | "NH" | "CA" | "AZ", "warm" | "cold">': NH, AZ
types.d.ts(639, 124)

It wants us to specify all the states in the enum, which is not intuitive or useful, and is inconsistent with the Zod parser.

@scotttrinh
Copy link
Collaborator

The current behavior matches TypeScript's Record type, so I think that even if that feels unintuitive or unergonomic, it's the right way to go.

@jeremybull
Copy link
Contributor Author

@scotttrinh OK, but how do you feel about the inconsistency between how zod parses, and how the inferred typescript interface checks the same record?

@scotttrinh
Copy link
Collaborator

@jeremybull

Ahh, I see what you mean, I missed that part of the issue in my first read. Yeah, we should align the runtime and compile time behavior, I think. @colinhacks this feels like a breaking change since it causes schemas to fail that previously succeeded. Any thoughts or suggestions about what to do here?

@jeremybull
Copy link
Contributor Author

jeremybull commented Nov 8, 2021

@scotttrinh @colinhacks One option might be to accept and document the breaking change as a bug fix for the inconsistency (enum record keys have only been available since 3.9 and are not yet well documented, so won't be widely adopted). Then we could add a partial() method to the record schema so anyone like me who wants the old parsing behavior can still get it.

@jeremybull
Copy link
Contributor Author

jeremybull commented Nov 11, 2021

@scotttrinh I'm not familiar with your release process/schedule... what kind of timeframe should I expect for you making a decision on this? (If you want to go a different route, I could put together an alternative PR.)

I already used zod in our app before realizing it doesn't work for our enum-keyed maps. Just trying to decide whether to stick with my patched fork for now, or look for a different workaround...

Thanks!

@scotttrinh
Copy link
Collaborator

@jeremybull

@colinhacks is the lead developer of Zod and is typically the one who cuts releases, so I'll defer to him on both the timing and likelihood of making this change. I think sticking with your fork is fine for now, but it's completely possible that we "fix" (or not?) it a different way, so definitely make the choice that makes the most sense for you and your project.

@JacobWeisenburger JacobWeisenburger added the enhancement New feature or request label Feb 27, 2022
@xajhqffl
Copy link

xajhqffl commented Mar 1, 2022

Typescript has support for using partial with record. If we can add partial to z.record, it would be great. And this won’t be breaking changes.

@bengry
Copy link

bengry commented Jan 8, 2024

Just came across this, and it seems like if you use z.nativeEnum the behavior is not the same - it results in Record<SomeEnum, SomeValue> and not Partial<Record<SomeEnum, SomeValue>>, as it should. Or am I missing something?

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

Successfully merging a pull request may close this issue.

5 participants