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

Make validators accept ZodEffects to handle refined schemas #47

Merged
merged 1 commit into from
Sep 19, 2023
Merged

Make validators accept ZodEffects to handle refined schemas #47

merged 1 commit into from
Sep 19, 2023

Conversation

esafak
Copy link
Contributor

@esafak esafak commented Sep 19, 2023

@changeset-bot
Copy link

changeset-bot bot commented Sep 19, 2023

🦋 Changeset detected

Latest commit: eaf4f75

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
formsnap Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@vercel
Copy link

vercel bot commented Sep 19, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
formsnap ✅ Ready (Inspect) Visit Preview 💬 Add feedback Sep 19, 2023 3:06am

@huntabyte
Copy link
Member

Thanks for this! @esafak

@huntabyte huntabyte merged commit 176231e into svecosystem:main Sep 19, 2023
@github-actions github-actions bot mentioned this pull request Sep 19, 2023
@esafak
Copy link
Contributor Author

esafak commented Sep 19, 2023

I fear this might not have fixed it. Apologies for not testing carefully. I'll take another look.

@huntabyte
Copy link
Member

It fixed another discrepancy though! I think I know why this is happening @esafak, but I'm not 100% sure of how to resolve it elegantly. It's with the ZodValidation<AnyZodObject> generic type on the form.svelte component. I can try to look into it further later today if you aren't able to find anything before then!

@esafak
Copy link
Contributor Author

esafak commented Sep 19, 2023

Please do, you know your library best. A unit test for this case would be welcome. Also, this is recognized as a bug in Zod:

colinhacks/zod/issues/2474
colinhacks/zod/issues/2646

@BenocxX
Copy link
Contributor

BenocxX commented Sep 23, 2023

Is the issue #45 fixed?

Thanks for the work :)

@BenocxX
Copy link
Contributor

BenocxX commented Sep 24, 2023

I confirm that the fix did not work. I still get an error message when I pass config to <Form.Field>.

@esafak
Copy link
Contributor Author

esafak commented Sep 28, 2023

I confirm that the fix did not work. I still get an error message when I pass config to <Form.Field>.

Can you try #52 ?

@BenocxX
Copy link
Contributor

BenocxX commented Sep 28, 2023

How can I try the new version in my existing project? Can I tell my package.json to use a specific branch for FormSnap?

@esafak
Copy link
Contributor Author

esafak commented Sep 29, 2023

Remove your existing formsnap, and install from https://github.com/esafak/formsnap/tree/refine: https://stackoverflow.com/questions/17509669/how-to-install-an-npm-package-from-github-directly

That is not how I did it, though. I cloned it installed it using (p)npm link: https://docs.npmjs.com/cli/v9/commands/npm-link

But the first way should be easier.

@BenocxX
Copy link
Contributor

BenocxX commented Sep 30, 2023

It seems to be working as expected! I have a refine on the schema, everything works and no TS error.

Thanks a lot for the fix, I really needed this!

Though, I found an other problem related to refine. When having more complex rules for fields (coerce and nativeEnum), the refine isn't called if they fail. This issue might be related to Zod more than FormSnap though.

Here's the schema:

export const registerSchema = z
  .object({
    email: z.string().email(),
    password: z.string().min(8),
    confirmPassword: z.string(),
    firstName: z.string().min(1),
    lastName: z.string().min(1),
    gender: z.nativeEnum(Gender), // If fails validation, stops the refine from being executed
    birthDate: z.coerce.date(), // If fails validation, stops the refine from being executed
  })
  .refine((data) => data.confirmPassword === data.password, {
    message: 'Passwords do not match',
    path: ['confirmPassword'],
  });

@esafak
Copy link
Contributor Author

esafak commented Sep 30, 2023 via email

@BenocxX
Copy link
Contributor

BenocxX commented Oct 1, 2023

Yeah it seems to be the same issue. Ergh sadly it doesn't look like the Zod devs will be fixing it anytime soon...

Thanks for the help!

@wellitonscheer
Copy link

wellitonscheer commented Oct 16, 2023

hi, i did this just to try and it looks fine.

import { z } from "zod";
import { validateVal } from "packages/utils/validateVal";

const val = z.string().refine(
  (val) => {
    return validateVal(val);
  },
  { message: "Invalid val" }
);

export const MySchema = z.object({
  id: z.string(),
  name: z.string(),
  val: val,
});
export type MyType = z.infer<typeof MySchema>;

i have no idea if its perfect but its working for now.
tell me if didnt work for you guys, it will be useful for me too.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Impossible to assign "config" to Form.Field when the Zod schema has a "refine"
4 participants