zod icon indicating copy to clipboard operation
zod copied to clipboard

Add `z.string().boolean()` function to parse a boolean value represented by string logically

Open jlandowner opened this issue 1 year ago • 9 comments

Close #2985 Close #1630

I thought which function name to use in the followings, but use the original name for now.

  • z.string().boolean()
  • z.string().toBoolean()

jlandowner avatar Nov 24 '23 05:11 jlandowner

Deploy Preview for guileless-rolypoly-866f8a ready!

Built without sensitive environment variables

Name Link
Latest commit d71588cf74b4bee35828bd6489dd5371841b19ac
Latest deploy log https://app.netlify.com/sites/guileless-rolypoly-866f8a/deploys/662fab9654d4a00008ecbc8a
Deploy Preview https://deploy-preview-2989--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 configuration.

netlify[bot] avatar Nov 24 '23 05:11 netlify[bot]

This is a brilliant feature improvement. I have wasted a lot of time trying to figure out why my code didn't work as it was because Zod by default parses "false" to be true when using a Boolean. That is a foot gun that I am sure a lot of people run into if you are parsing environment variables or URL query parameters. It would be great to see this merged in.

bhouston avatar Jan 15 '24 15:01 bhouston

Also I should add that this bug isn't present with Joi. So when I converted over to Zod from Joi, this was a huge surprise.

bhouston avatar Jan 15 '24 15:01 bhouston

@bhouston Thanks for comments! I'm still waiting for a review but it seems maintainer is busy.

jlandowner avatar Jan 15 '24 16:01 jlandowner

This is a brilliant feature improvement. I have wasted a lot of time trying to figure out why my code didn't work as it was because Zod by default parses "false" to be true when using a Boolean. That is a foot gun that I am sure a lot of people run into if you are parsing environment variables or URL query parameters. It would be great to see this merged in.

I was also facing the same issue, @bhouston , but I fixed that by doing this:

Previous: //wrong output

const schema = z.coerce.boolean();
schema.parse("true"); // => true
schema.parse("false"); // => true

Expected: //correct output

const schema = z.coerce.string().transform((val) => val === "true")
schema.parse("true"); // => true
schema.parse("false"); // => false

hamzasial1911 avatar Mar 01 '24 16:03 hamzasial1911

Hi @hamzasial1911 This PR is intented to solve general usecases not only "true" and "false".

You can see all the boolean strings this PR supported in https://github.com/colinhacks/zod/issues/2985

The workarounds are discussed in https://github.com/colinhacks/zod/issues/1630

jlandowner avatar Mar 02 '24 01:03 jlandowner

I had to do the transform right now and I was looking for something that was already part of the library, glad we got this PR open to tackle this! ty @jlandowner

JCMais avatar Mar 18 '24 21:03 JCMais

When can we expect this pull request to be merged?

rtatton avatar Mar 26 '24 16:03 rtatton

Hi @noook. Thank you for your review! I took all your suggestions. Now I am working on resolving conflicts and catch up to the latest commit.

jlandowner avatar Apr 29 '24 14:04 jlandowner

This is a nice DX improvement but thoughts on changing the method name? z.string().boolify()?

Although the native JS implementation of Boolean() might not be a very useful api for our domain

// Boolean('true') === true
// Boolean('false') === true
// Boolean('') === false

When I see zod.coerce.boolean(), I'd still expect it to be consistent with the other coercion methods which all just wrap the built-in JS constructors)

araphiel avatar Apr 30 '24 21:04 araphiel

Instead of hardcoding why not allow users to pass a predicate to choose which values represent boolean values?

nikolalukovic avatar Apr 30 '24 21:04 nikolalukovic

Hi @araphiel. Thank you for your feedback. I can change the function name soon!

Here is the current options.

  1. z.string().boolean()
  2. z.string().toBoolean()
  3. z.string().boolify()

Currerntly No.1 but I thought No.2 because there are already string transformers which names are toXXX().

https://github.com/colinhacks/zod?tab=readme-ov-file#strings

// transforms
z.string().trim(); // trim whitespace
z.string().toLowerCase(); // toLowerCase
z.string().toUpperCase(); // toUpperCase

Do you think boolify() is better than toBoolean() for JS users?

jlandowner avatar Apr 30 '24 23:04 jlandowner

I think either choices work well but it might be worth waiting for @colinhacks input before implementing this change

araphiel-nf avatar Apr 30 '24 23:04 araphiel-nf

Hi @nikolalukovic . Thank you for your feedback!

It is from the Go's strconv.ParseBool() I referenced. https://cs.opensource.google/go/go/+/refs/tags/go1.22.2:src/strconv/atob.go;l=10

Instead of hardcoding why not allow users to pass a predicate to choose which values represent boolean values?

It is great. What you are envisioning like this?

interface:

- boolean(options?: { message?: string }) {
+ boolean(options?: { message?: string, trueValues?: string[], falseValues?: string[]}) {

example usage:

z.strings().boolean() // use hardcoded default patterns.
z.strings().boolean({trueValues=["yes", "1"], falseValues=["no", "0"]}) // override value patterns.

jlandowner avatar Apr 30 '24 23:04 jlandowner

A sane choice would be to use all the values that Postgres accepts. https://www.postgresql.org/docs/devel/datatype-boolean.html

nicksrandall avatar May 01 '24 03:05 nicksrandall

@jlandowner That's a lot better, I'd even go for something like this but it's your choice in the end which one you want to implement:

- boolean(options?: { message?: string }) {
+ boolean(options?: { message?: string, truePredicate?: (trueValue: string) => boolean, falsePredicate?: (falseValue: string) => boolean }) {

nikolalukovic avatar May 01 '24 15:05 nikolalukovic

I don't like the idea of Zod being opinionated about which string literals mean true. That said, it's clear that parsing boolean env vars is a little fiddly/verbose right now, and probably requires relatively advanced features like .catch().

My current proposal to address this use case is given here: https://twitter.com/colinhacks/status/1785401238328320283

const env = z.literal(["t", "true", "on"], { case: "insensitive" }).success();

// true
env.parse("t");
env.parse("true");
env.parse("TRUE");
env.parse("oN");

// false
env.parse("f");
env.parse("false");
env.parse(0);
env.parse(1);
env.parse(true);
env.parse(false);
env.parse("tru");
env.parse("off");

This requires a few new features to be implemented.

  1. allow arrays of literals in z.literal()
  2. support a case sensitivity flag
  3. .success() method that returns true when validation passes and false otherwise

All of these are slated to be implemented/released in Zod 4. I'm going to close this, but anyone should feel free to chime in with feedback on the proposed alternative.

colinhacks avatar May 03 '24 00:05 colinhacks

Hi @colinhacks Thank you for your feedback. I look forward to Zod 4. And then if the best practice for logical parsing of booleans is documented, it would help a lot of people.

jlandowner avatar May 03 '24 01:05 jlandowner