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

Support type inference for ZodString #1730

Closed
wants to merge 1 commit into from

Conversation

tachibanayui
Copy link

We could use Typescript template literal to narrow the type provided by z.infer<T>. This is especially useful when working with discriminated unions where part of the discriminator is dynamic. For example, the type should be ​`Hello${string}world!` | undefined instead of string | undefined:

 const zstr = z.string()
  .startsWith("Hello")
  .endsWith("world!")
  .optional();

type ZStr = z.infer<typeof zstr>; // `Hello${string}world!` | undefined

// Scenario 1: Works fine
const data: ZStr = "Hello world!"; 
zstr.parse(data);

// Scenario 2:
const data: ZStr = "I don't follow rules!"; // ts(2322): Type '"I don't follow rules!"' is not assignable to type '`Hello${string}world!`
zstr.parse(data); // Error

Here is an example of discriminated unions:

const ZCat = z.object({
    discrimiator: z.string().startsWith("cat"),
    claws: z.array(z.string().uuid()),
})
const ZBird = z.object({
    discrimiator: z.string().startsWith("bird"),
    wings: z.array(z.string().uuid()),
});

const ZAnimal = z.union([ZCat, ZBird]);
type Animal = z.infer<typeof ZAnimal>;
const catWithWings: Animal = {
    discrimiator: "cat/abyssinian",
    // Oh no! Typescript think our cat have wings is valid!
    wings: [faker.datatype.uuid()]
};

// Zod will catch the error
ZAnimal.parse(catWithWings) 

Here's an example after these changes:

// ... zod definitions above
type Animal = z.infer<typeof ZAnimal>;
const catWithWings: Animal = {
    discrimiator: "cat/abyssinian",
    // ts(2232): Type '{ discrimiator: "cat/abyssinian"; wings: string[]; }' is not assignable to type
    // '{ discrimiator: `cat${string}`; claws: string[]; } | { discrimiator: `bird${string}`; wings: string[]; }'.
    wings: [faker.datatype.uuid()],
};

While we are at it, I think it is beneficial to implement a method that checks for a substring. I will call it includes for now for its similarity with Javascript standard library.

implement method `includes()` for `ZodString`
@netlify
Copy link

netlify bot commented Dec 20, 2022

Deploy Preview for guileless-rolypoly-866f8a ready!

Built without sensitive environment variables

Name Link
🔨 Latest commit 1c3ba90
🔍 Latest deploy log https://app.netlify.com/sites/guileless-rolypoly-866f8a/deploys/63a1bdf8fca15d0008e1420c
😎 Deploy Preview https://deploy-preview-1730--guileless-rolypoly-866f8a.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@colinhacks
Copy link
Owner

Thanks for the PR and cool concept, but I'm not sure about it. IMO it's not worth muddying the type definition of ZodString with a bunch of generics. It's a big hit to the readability of the type signature.

Screenshot 2022-12-24 at 12 39 02 AM

Also narrowing the type signature like this would potentially break a lot of existing code and might be unexpected.

const schema = z.object({
  name: z.string().startsWith("asdf").endsWith("qwer"),
});
type Schema = z.infer<typeof schema>;

function doStuff(_: Schema) {}

const value = { name: "asdf_qwer" };
doStuff(value);
// Argument of type '{ name: string; }' is not assignable to parameter of type '{ name: StartMidEndParts<"asdf", "", "qwer">; }'.

I think template literals are sufficiently advanced that users should stick to using z.custom if they need them.

@tachibanayui
Copy link
Author

tachibanayui commented Dec 24, 2022

Thanks for the awesome feedback! For readability, I think it is convenient to see type parameters when the user calls methods such as .startsWIth(), .endsWith(), and .includes(). But I agree that it shouldn't be visible if the user doesn't use those methods. For that, we can use:

// ... Rename the old ZodString to ZodSubString. I would like to rename StartMidEndParts to SubString

type MapZSubString<T> = T extends (...args: infer P) => NoSubString ? (...args: P) => ZodString : T
type NoSubString = ZodSubString<"", "", "">
type Excludes = "startsWith" | "endsWith" | "includes"
type ZodString = {
    [TProperty in keyof NoSubString]: TProperty extends Excludes
        ? NoSubString[TProperty]
        : MapZSubString<NoSubString[TProperty]>;
} & {
    _parse(input: zod.ParseInput): zod.ParseReturnType<string>;
};

static create = (params?: RawCreateParams & { coerce?: true }): ZodString => {
  return new ZodSubString({
    checks: [],
    typeName: ZodFirstPartyTypeKind.ZodString,
    coerce: params?.coerce ?? false,
    ...processCreateParams(params),
  }) as ZodString;
};

After these changes, if the user doesn't call the method mentioned above, ZodString will be returned, otherwise ZodSubString<TStart, TMid, TEnd>

// const x: ZodString
const x = z.string().max(100)
// const y: ZodSubString<"hello", "", "">
const y = z.string().startsWith("hello")
// const yx: ZodSubString<"hello", "", "">
const yx = z.string().startsWith("hello").max(100);

For type narrowing that might cause issues, I think the input should be validated before giving it to the function doStuff(...). And fortunately schema.parse() happens to convert unknown to our desired type:

const value = schema.parse({ name: "asdf_qwer" });
doStuff(value);

In case the user absolutely wants to use the input without validation, they can use the as keyword:

const value = { name: "asdf_qwer" } as Schema;
doStuff(value);

People use zod because it is meant to be a pleasant way to validate data, but writing z.custom<`prefix${string}`&`${string}suffix`>((val) => z.string().startsWith("prefix").endsWith("suffix").parse(val)) kinda defeat the purpose. But that is my own opinion, please let me know what you think. If you agree with me, let me know, and I will implement the changes mentioned above.

EDIT: I entirely forgot about existing code bases that potentially use the pattern you described. To avoid breaking changes, we could create methods specifically for templated literal: startsWithLiteral(), endsWithLiteral(), and includesLiteral() or add a 3rd argument exact?: true to avoid breaking changes thanks @santosmarco-caribou. However, I wish these methods be the default in the next major release.

@igalklebanov
Copy link
Contributor

igalklebanov commented Jan 4, 2023

startsWithLiteral(), endsWithLiteral(), and includesLiteral() or add a 3rd argument exact?: true

I strongly believe #1786 can be a great alternative to these (they add complexity, will confuse people, and will break "what you write is what you get" - I used string() so I probably want the type to be string in the end).

@stale
Copy link

stale bot commented May 10, 2023

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 stale bot added the stale No activity in last 60 days label May 10, 2023
@stale stale bot closed this Jun 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stale No activity in last 60 days
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants