zod icon indicating copy to clipboard operation
zod copied to clipboard

Fix issue #1611

Open john-schmitz opened this issue 2 years ago • 2 comments

closes #1611

I quickly drafted a working solution so we could iterate and discuss its design.

I also added the same proposed behavior to strings as well, since both strings and arrays share the .length function.

Looking further into the codebase, I think it would be ideal to create a new check (something like "exact_legth") and a ZodIssueCode so we can share the same locale behavior between Arrays and Strings.

Whats your take? How should I integrate those changes into Zod's current codebase?

john-schmitz avatar Nov 30 '22 11:11 john-schmitz

Deploy Preview for guileless-rolypoly-866f8a ready!

Name Link
Latest commit 36396bb099d979b16d9dc686fb3e9ca442a459b0
Latest deploy log https://app.netlify.com/sites/guileless-rolypoly-866f8a/deploys/6396782a29a13b0008c7327e
Deploy Preview https://deploy-preview-1620--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.

netlify[bot] avatar Nov 30 '22 11:11 netlify[bot]

Whats your take? How should I integrate those changes into Zod's current codebase?

IMO this seems very sensible - especially since there's no API changes and seems in line with the current way of organizing error messages. Just my $0.02 USD. Thanks for the pr @john-schmitz !

maxArturo avatar Dec 06 '22 17:12 maxArturo

Glad to help. The impact Zod had on my workflow is immense. Always good to give back.

john-schmitz avatar Dec 06 '22 22:12 john-schmitz

Hmm... I'm not sure if I like this solution. (but correct me if I'm wrong)

You are setting the default message directly on the method, promoting it to the highest priority level when generating the final message.

Say, for example, I've decided to define my error map on the .parse() call:

const myArraySchema = z.array(z.string()).length(2).parse(["foo"], {
  errorMap: (issue) => {
    if (issue.code === "too_small") {
      return { message: "Hey yo, the array must have a length of 2." };
    }
    return { message: "Nope" };
  },
});

Since I'm not setting the "Hey yo, the array must have a length of 2." message in the .length(2) call, it will be overwritten by your default message because messages set directly on the check ('length') call have the highest priority (I'm not setting it there, but you are).

Default messages should always have the lowest priority.

Can you please add a test of the above code and see the result?

santosmarco-caribou avatar Dec 10 '22 04:12 santosmarco-caribou

As @santosmarco-caribou says, the original approach does break Zod's error message hierarchy. I pushed a commit that uses a different approach, adding an optional exact flag (alongside inclusive) that we can use to customize the default error message here. Achieves the goal in a backwards compatible way.

colinhacks avatar Dec 12 '22 00:12 colinhacks