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

Add more accurate types to Zod helpers #346

Closed
wants to merge 2 commits into from
Closed
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 37 additions & 8 deletions packages/zod-form-data/src/helpers.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { setPath } from "set-get";
import {
input,
z,
ZodArray,
ZodEffects,
Expand All @@ -10,10 +11,23 @@ import {
ZodTypeAny,
} from "zod";

type ExtendsDefaultType<
DefaultType extends ZodTypeAny,
ProvidedType extends ZodTypeAny
> = (
ProvidedType extends ZodType<any, any, infer Input>
? Input extends input<DefaultType>
? ProvidedType
: never
: never
) extends never
? never
: ProvidedType;

type InputType<DefaultType extends ZodTypeAny> = {
(): ZodEffects<DefaultType>;
<ProvidedType extends ZodTypeAny>(
schema: ProvidedType
schema: ProvidedType & ExtendsDefaultType<DefaultType, ProvidedType>
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

): ZodEffects<ProvidedType>;
};

Expand All @@ -25,6 +39,21 @@ const preprocessIfValid = (schema: ZodTypeAny) => (val: unknown) => {
return val;
};

/**
* Provides a version of `z.preprocess` that does not broaden the allowable input type of the
* returned ZodEffects from `input<T>` to `unknown`. This is important for preserving type input
* information for e.g. default values, or for the input types of `parse()` functions.
*
* This should be used when the preprocess function is not intended to broaden the allowable input
* types from those of the provided schema.
*
* See: https://github.com/colinhacks/zod/pull/1752
*/
const preprocessWithoutUnknown = <T extends ZodTypeAny>(
preprocessFn: Parameters<typeof z.preprocess>[0],
schema: T
): ZodEffects<T> => z.preprocess(preprocessFn, schema);

/**
* Transforms any empty strings to `undefined` before validating.
* This makes it so empty strings will fail required checks,
Expand All @@ -33,7 +62,7 @@ const preprocessIfValid = (schema: ZodTypeAny) => (val: unknown) => {
* If you want to customize the schema, you can pass that as an argument.
*/
export const text: InputType<ZodString> = (schema = z.string()) =>
z.preprocess(preprocessIfValid(stripEmpty), schema) as any;
preprocessWithoutUnknown(preprocessIfValid(stripEmpty), schema);
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.


/**
* Coerces numerical strings to numbers transforms empty strings to `undefined` before validating.
Expand All @@ -42,7 +71,7 @@ export const text: InputType<ZodString> = (schema = z.string()) =>
* If you want to customize the schema, you can pass that as an argument.
*/
export const numeric: InputType<ZodNumber> = (schema = z.number()) =>
z.preprocess(
preprocessWithoutUnknown(
preprocessIfValid(
z.union([
stripEmpty,
Expand All @@ -53,7 +82,7 @@ export const numeric: InputType<ZodNumber> = (schema = z.number()) =>
])
),
schema
) as any;
);

type CheckboxOpts = {
trueValue?: string;
Expand Down Expand Up @@ -83,10 +112,10 @@ export const checkbox = ({ trueValue = "on" }: CheckboxOpts = {}) =>
]);

export const file: InputType<z.ZodType<File>> = (schema = z.instanceof(File)) =>
z.preprocess((val) => {
preprocessWithoutUnknown((val) => {
//Empty File object on no user input, so convert to undefined
return val instanceof File && val.size === 0 ? undefined : val;
}, schema) as any;
}, schema);

/**
* Preprocesses a field where you expect multiple values could be present for the same field name
Expand All @@ -97,11 +126,11 @@ export const file: InputType<z.ZodType<File>> = (schema = z.instanceof(File)) =>
export const repeatable: InputType<ZodArray<any>> = (
schema = z.array(text())
) => {
return z.preprocess((val) => {
return preprocessWithoutUnknown((val) => {
if (Array.isArray(val)) return val;
if (val === undefined) return [];
return [val];
}, schema) as any;
}, schema);
};

/**
Expand Down
Loading