colinhacks / zod

TypeScript-first schema validation with static type inference
https://zod.dev
MIT License
31.73k stars 1.1k forks source link

z.string() validates empty strings #2466

Open mikeybinns opened 1 year ago

mikeybinns commented 1 year ago

This is created after viewing the conversation on this issue: #63

Currently, the result of z.string() validation of an empty string "" leads to a pass instead of a fail, even though the field itself is required. This is not documented anywhere except github issues and apparently an old changelog, but it is not very intuitive in some cases and leads to potential issues especially when trying to validate forms.

Potential solutions:

  1. Update z.string() to not pass validation for "" unless .optional() is provided (breaking change)
  2. Update the zod strings documentation section with a note specifically about this functionality, providing the alternative proposed solution. specifically:
    z.string().trim().min(1, { message: "Required" })
    // or
    const notEmpty = z.string().trim().min(1, { message: "Required" });
    z.string().pipe(notEmpty);

    Credit for the above solutions go to the folks in issue #63

I expect to get resistance to option 1 so hopefully option 2 is a reasonable middle ground 🙂

scotttrinh commented 1 year ago

Currently, the result of z.string() validation of an empty string "" leads to a pass instead of a fail, even though the field itself is required.

I think this is a common misunderstanding of what the purpose of Zod is. Zod is here to represent the type system of TypeScript at runtime. So "required" and "string" in these contexts have nothing to do with similar concepts in, for instance, HTML Forms. You can imagine other use cases that have other concepts of what a string is: maybe it has a maximum length? A certain encoding like utf8? But, for Zod, a z.string() is value that matches a typescript string.

const str: string = ""; // OK
// Other "empty"-like values
const num: number = 0; // OK
const rec: Record<string, string> = {}; // OK
// etc.

Because ☝️ is true, z.string() should match that behavior. IMO, form integrations should do the heavy lifting of mapping the HTML Form concept of "required" to TypeScript's such that they automatically trim and convert empty strings to null or undefined or whatever makes the most sense for that use case.

This is not documented anywhere except github issues and apparently an old changelog

I disagree since the existing string documentation already has many examples of showing both the trim feature, and the min feature that the proposed solution is based on, but feel free to submit a PR to add z.string().trim().min(1) to the list of transformation examples.

mikeybinns commented 1 year ago

After reading what you've written, I agree with everything you've said there about how Zod should work, I guess the issue is purely one of expectations, rather than functionality.

Perhaps instead of just adding an example, I think a better option may be to add a section in "Guides and concepts" specifically about forms and what you've just written, so people using zod for form validation (a pretty common use case from my understanding) will approach it with the correct frame of mind.

I'll draft up a PR and send it over shortly for review, thanks for your clear explainer @scotttrinh 😃

Derek-Stonks commented 1 year ago

@scotttrinh I agree that the proposed solutions were antithetical to Zod's purpose, but I disagree with the sentiment that there is no problem. The first sentence of the introduction in the README reads:

Zod is a TypeScript-first schema declaration and validation library.

While I understand that it is not the focus to cover all validation use cases, I would bet that near half of all form validation is just ensuring that the field has been filled out. So why is it that Zod can give me string validation for emails or urls with reasonable error messages, but for every required field I have to add .trim().min(1, { message: "Required" })?

This really should be addressed, and I'm sure that there are solutions that wouldn't interfere with Zod's guiding principles. Off the top of my head, why not just have a isNotEmpty method on stringTypes?

WillsterJohnson commented 1 year ago

@Derek-Stonks it seems that an isNotEmpty already exists in the form of .nonempty() (https://github.com/colinhacks/zod/blame/c5763112e2912390f3317d738e4261fa8747494e/src/types.ts#L976), though it was depreacted in favor of using .min(1), both of these have the same default error message; 'String must contain at least 1 character(s)'

scotttrinh commented 1 year ago

@mikeybinns

Perhaps instead of just adding an example, I think a better option may be to add a section in "Guides and concepts" specifically about forms and what you've just written, so people using zod for form validation (a pretty common use case from my understanding) will approach it with the correct frame of mind.

Yeah, form validation is a common pain point with getting started with Zod, and for that matter React, and TypeScript! Thanks for #2467 , I think that strikes the right tone and hopefully can help others who are struggling to use Zod with forms without a form integration.

@Derek-Stonks

Off the top of my head, why not just have a isNotEmpty method on stringTypes?

For my own use cases, I would disagree that " " should be treated as "empty", and I think consumers should be explicit about what their expectations are around that. In general, once Zod starts having an opinion about what a domain-specific concept like "empty" means, we take on the burden of defending that position against people who have different expectations. I think that's why we've tended toward very explicit built-in validators and every time we've deviated from that (emoji 😣 ) we've been bitten (#2228) by it.

205g0 commented 11 months ago

If you don't deal every day with forms and come back to some old codebase like I did today you just forgot about this oddity and might waste hours why something isn't working.

Even if this was documented it'd be still unintuitive. Even when technically right (zod reflects only TS types), an empty string is often used as a false-y thing (I know that it does not equal undefined), but b/c of Boolean("") === false it's often used as a condition for something not to happen. So, while an empty string isn't undefined it's treated in 99% as such, yeah there are edge cases and now, we can split hairs what's right or wrong, it's about how the majority treats "" in the wild.

Moreover, now you have .optional() fields and fields with non-empty strings requirements with .trim().min(1, ( message: "required" }) which doesn't feel like consistent nor elegant code.

@colinhacks what's your take on this?

tauhid97k commented 8 months ago

Can anyone tell me how I can validate an empty string as "required" with min(1) and, at the same time, "password must be at least 8 characters" with min(8)?

mikeybinns commented 8 months ago

Can anyone tell me how I can validate an empty string as "required" with min(1) and, at the same time, "password must be at least 8 characters" with min(8)?

Just use min(8) 8 is at least 1 so it covers both.

TheMikeyRoss commented 8 months ago

Can anyone tell me how I can validate an empty string as "required" with min(1) and, at the same time, "password must be at least 8 characters" with min(8)?

Just use min(8) 8 is at least 1 so it covers both.

But in this case I can't have 2 different messages as:

(when field is empty) = 'Password is required (when field is less than 8) = 'Password is too short'

How can I display a different message for each condition?

zaaakher commented 8 months ago

@tauhid97k @TheMikeyRoss You can do both

password: z
    .string({ required_error: "Password is required" })
    .min(1, { message: "You must enter a password" })
    .min(8, { message: "Passowrd is too short" })
TheMikeyRoss commented 8 months ago

@tauhid97k @TheMikeyRoss You can do both

password: z
    .string({ required_error: "Password is required" })
    .min(1, { message: "You must enter a password" })
    .min(8, { message: "Passowrd is too short" })

Thanks @zaaakher that worked exactly how I wanted

Theo-flux commented 2 months ago
const lowerCaseRegex = /(?=.*[a-z])\w+/;
const upperCaseRegex = /(?=.*[A-Z])\w+/;
const numberRegex = /\d/;
const specialCharcterRegex = /[`!@#$%^&*()_+\-=[\]{};':"\\|,.<>/?~]/;

password: z
      .string({ required_error: 'Password is required' })
      .min(8, { message: 'Must be at 8 or more characters long.' })
      .refine((value) => upperCaseRegex.test(value), 'Password must contain atleast an uppercase.')
      .refine((value) => numberRegex.test(value), 'Password must contain atleast a number.')
      .refine(
        (value) => specialCharcterRegex.test(value),
        'Password must contain atleast a special character.'
      )
Davilink commented 1 month ago

I was also expecting that z.string() to enforce non null-ish AND no empty string, so for me the solution 1 also make more sense.