Add more accurate types to Zod helpers

[fnimick]

@airjp73 I did some poking and wasn't sure how to set up the monorepo environment for testing, but I'm 99% sure the type changes in the first commit won't cause any problems. This is simply a readability and type-safety change to remove the need for any casts in the helper functions which use z.preprocess.

For example, in the current setup, doing the following does not produce a type error:

export const text: InputType<ZodString> = (schema = z.number()) =>
  preprocessWithoutUnknown(preprocessIfValid(stripEmpty), schema) as any;

though it should as z.number() does not generate a ZodString input.

By default, z.preprocess broadens the input type of the resulting ZodEffect from the input type of the provided schema to unknown. This can be valuable, but in the cases of these helpers, the input types should remain the same.

By defining and using a version of preprocess which does not perform this broadening, we can avoid the need for the as any casts in all of the helper functions.

I have adapted some of the preprocessors to use in my own projects (sveltekit instead of remix) and ended up needing to refine these types in order to accurately and safely use them with additional preprocessing steps. Figured I'd contribute these fixes back!

---

The second commit is substantially more risky, but possibly valuable. This modifies InputType to perform type checking on the provided schema, by asserting an intersection between the default schema from an InputType and the provided schema. Previously, e.g. text(z.number()) did not provide a type error, despite the fact that string is not assignable to number. Now, this will throw an error.

I did some basic tests to confirm that e.g. all modifications that allow string to be assigned to the input generic parameter of a ZodType are legal arguments to text(), e.g. text(z.string().optional()) works as expected. Similarly, z.number().nullable() is a valid imput to numeric(). However, I'm not enough of a Typescript expert to state this unequivocally - so please give me feedback on this approach!

EDIT: please note that there are now type errors in the tests, which are testing e.g. how text(z.number()) and numeric(z.string()) behave. I believe these should be type errors, since it doesn't really make sense to me to have fields that coerce certain types of strings also accept non-string schemas, or fields that coerce to number accept non-number schemas.

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
remix-validated-form	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Dec 9, 2023 0:24am

[airjp73]

Hi!

Thanks for putting the time into making a contribution. :)

For a few reasons, I don't think I can merge this PR, but I did want to take the time to leave a few notes (here and in other comments) that I hope are helpful!

---

EDIT: please note that there are now type errors in the tests, which are testing e.g. how text(z.number()) and numeric(z.string()) behave.

In those cases, you can add a // @ts-expect-error comment to the line before, which is really useful for testing scenarios that are disallowed by the types.

[airjp73]

In generic-heavy library code, using as any doesn't have the same ickiness that it does in normal app code. This is because TS sometimes has a hard time figuring out what's really safe when there are a lot of type parameters.

The focus of the types is to provide a robust contract to the library consumer rather than keeping everything inside the library very strict. So there's often some fudging that happens between the library internals and the externally exposed types.

preprocessWithoutUnknown is still fudging the types, but in a more complicated way (and an extra layer of indirection) just to avoid using any. For that reason, I'd like to stick with the as any here.

[airjp73]

There are two main things to call out here:

1.

This might over-limit the schema's we allow. I made a Typescript Playground to help demonstrate.

It covers the main use-cases pretty well, but I think it's reasonable to want something like z.coerce.date to work with z.text. Unfortunately, z.coerce uses the wrong input type on purpose (reference), so I'm not sure how much we can work around that.

2.

It's possible to accomplish this in a simpler way. Since InputType is an internal type function (and not exposed as an API), we can freely change the inputs. Here's another typescript playground demonstrating an alternate take on this idea. The result appears to be equivalent (unfortunately still having issues with z.coerce).