fabian-hiller / valibot

The modular and type safe schema library for validating structural data 🤖
https://valibot.dev
MIT License
6k stars 186 forks source link

Add an advanced `refine` method (similar to Zod's `superRefine`) #597

Closed ruiaraujo012 closed 3 months ago

ruiaraujo012 commented 4 months ago

Currently, there is no way to directly add custom/global issues to the validation, for example:

{
    "primaryEmail": "foo@email.com",
    "otherEmails": ["bar@email.com", "foo@email.com"]
}

To validate that the primaryEmail is not present in otherEmails there is no method that allows it, because I need some actions that add issues globally.

The forward action doesn't work because I cannot put the key and index of the duplicated email, in this case otherEmails.1.

One possible solution is a method that allow us to add issues to the list of issues already created by valibot validations. Something like:

v.someMethod((inputs, ctx) => {
// ...(my custom validation with ctx.addIssue())...
return ctx.issues // Or other type of return
}) 

Related: #487, #546

jansedlon commented 4 months ago

Just wanna say that this is the thing that would allow me to completely switch over to valibot :)

fabian-hiller commented 4 months ago

I plan to work on it next week 🙌

jansedlon commented 4 months ago

Awesome 🤩

ruiaraujo012 commented 4 months ago

Hello @fabian-hiller, from what I can tell, the codemod worked well to me, but I cannot finalize the migration because of this feature request. I don't want to rush you, I just wanted to know if you have an estimate of when it will be ready.

Thanks :)

fabian-hiller commented 4 months ago

Probably next week, as I should migrate some old docs pages first. Sorry for the delay.

ruiaraujo012 commented 4 months ago

It's ok, just to be aware ;)

fabian-hiller commented 4 months ago

What should we call this super action? One idea is to call it refine, similar to Zod, and another is to just call it action.

ruiaraujo012 commented 4 months ago

I'm not sure, refine seems to represent better what it does, but I like action 🙈

jansedlon commented 4 months ago

I'd call it refine

ruiaraujo012 commented 4 months ago

Yeah, refine is better.

fabian-hiller commented 4 months ago

action might make sense because it gives direct access to the underlying action API. refine might make sense as it has a stronger meaning and makes the code more readable. Do you have alternative names in mind?

jansedlon commented 4 months ago

Action gives me the impression of a single action or a single purpose thing but idk 😄

ruiaraujo012 commented 4 months ago

What about enhance? Not sure, but came to my mind.

fabian-hiller commented 3 months ago

I will think about it. Thank you for your feedback!

fabian-hiller commented 3 months ago

I like the names refine and enhance because they are short and match well with the names of the other actions. On the other hand, we distinguish between validation actions and transformation actions, and already support two actions with check and transform, which gives you full control over validation and transformation. Therefore it might make sense to add two functions instead of one as transformations are handled differently than validations.

For example, we could add two functions with the same name but a prefix in front. Since both give you access to the raw implementation a prefix like raw, super or power might make sense. So check and transform are the high level implementation with a nice DX and rawCheck and rawTransform are the low level version giving you full control over the dataset of the pipeline. What do you think of this idea?

ruiaraujo012 commented 3 months ago

The raw prefix is more self explanatory than refine or enhance and you're right, this method is no more than a check with an option to access the error layer and change it by adding new errors.

So I think the rawCheck and rawTransform would be a better name, even if less fancy than enhance or refine.

fabian-hiller commented 3 months ago

rawCheck and rawTransform are now available. Please give me feedback about the DX! There is no documentation yet, but here is a simple example showing the API.

import * as v from 'valibot';

const Schema1 = v.pipe(
  v.number(),
  v.rawCheck(({ dataset, addIssue }) => {
    if (dataset.typed && dataset.value <= 0) {
      addIssue({ message: 'Error!!!' });
    }
  })
);

const Schema2 = v.pipe(
  v.string(),
  v.rawTransform(({ dataset, addIssue, NEVER }) => {
    const length = dataset.value.length;
    if (length < 3) {
      addIssue({ message: 'Error!!!' });
      return NEVER;
    }
    return length;
  })
);
ruiaraujo012 commented 3 months ago

I will try soon, but I have a question, if I apply rawCheck to an object, what will be the field with the error? Is it possible to add an issue to a field?

fabian-hiller commented 3 months ago

You can still wrap rawCheck in our forward method or specify the path in addIssue by hand next to the message.

ruiaraujo012 commented 3 months ago

Oh, I didn't remember the forward.

Using the second options:

const Schema = v.pipe(
  v.object({
    email: v.pipe(v.string(), v.email()),
    password: v.pipe(v.string(), v.minLength(8)),
  }),
  v.rawCheck(({ dataset, addIssue }) => {
    if (dataset.typed && dataset.value.email.length < 10) {
      addIssue({
        input: dataset.value.email,
        message: 'Error!!!',
        path: [
          {
            input: dataset.value,
            key: 'email',
            origin: 'value',
            type: 'object',
            value: dataset.value.email,
          },
        ],
      });
    }
  }),
);

It's a bit bad in DX having to populate all those values in path

ruiaraujo012 commented 3 months ago

Otherwise, I find that while you don't have the items method, this work well:

const Schema = v.pipe(
  v.object({
    email: v.pipe(v.string(), v.email()),
    emails: v.array(v.pipe(v.string(), v.minLength(8))),
  }),
  v.rawCheck(({ dataset, addIssue }) => {
    if (dataset.typed) {
      const repeatedIndexes: number[] = [];

      dataset.value.emails.forEach((email, index) => {
        if (email === dataset.value.email) {
          repeatedIndexes.push(index);
        }
      });

      if (repeatedIndexes.length > 0) {
        repeatedIndexes.forEach((index) => {
          addIssue({
            input: dataset.value.emails,
            message: `Repeated ${index}`,
            path: [
              {
                input: dataset.value.emails,
                key: index,
                origin: 'value',
                type: 'array',
                value: dataset.value.emails[index],
              },
            ],
          });
        });
      }
    }
  }),
);

const result = v.safeParse(Schema, {
  email: 'jane@example.com',
  emails: ['jane1@example.com', 'jane@example.com', 'jane3@example.com'],
});

I'll try that with react-hook-forms soon

fabian-hiller commented 3 months ago

Thank you for your feedback!

It's a bit bad in DX having to populate all those values in path

I agree, but the other option would be to increase the bundle size and make it less powerful with less control by using a key path as with forward.

ruiaraujo012 commented 3 months ago

I can see that, it's ok, I think this method will be less used, so probably it won't hurt much. Thank you for this addition.

fabian-hiller commented 3 months ago

Thank you! Feel free to share more feedback in the long run. In the end, I am interested in the best solution and feedback helps me a lot to make the best decisions.

jansedlon commented 3 months ago

@fabian-hiller good job on the progress! One question, how does this play out with i18n in general? What about specifying custom error messages using i18next in different scenarios using this new function?

ruiaraujo012 commented 3 months ago

@fabian-hiller good job on the progress! One question, how does this play out with i18n in general? What about specifying custom error messages using i18next in different scenarios using this new function?

I usually create a namespace only for forms, and then use it in the schemas.

{
    "validation": {
        "email": "The email is not valid."
        ...
    },
    "required": {
        "email": "The email is required."
        ...
    },
}

Then in the schemas

const Schema = v.object({
    email: v.pipe(v.string(), v.nonEmpty('form:required.email'), v.email('form:validation.email'))
})

And then, using react I pass the message from valibot to t(message)

I could share a sandbox with this example if you want.

jansedlon commented 3 months ago

@ruiaraujo012 thank you for the example. Coming from zod-i18next, what's honestly truly the best is that you just pass params in addIssue where you define namespace and key and it's almost magically translated. If there was something close to this, it would be amazing. What do you think @fabian-hiller , could you look at that? Or I can try to find out how that params works exactly.

ruiaraujo012 commented 3 months ago

Oh, I see, I looked into zod-i18next, actually it'll be a nice addition. I think you could open an issue describing that and.

jansedlon commented 3 months ago

Will do :)

fabian-hiller commented 3 months ago

Have you read this guide? Paraglide JS looks nice. I am happy to answer questions about i18n.

jansedlon commented 3 months ago

@fabian-hiller Yeah, i know about Paraglide, however we don't really have the time to migrate to paraglide a i had some issues with it 🙈 We have (probably) thousands of translations keys right now and the migration would take a lot of time.

In valibot i use setSpecificMessage right now which is bounded to a specific validator, e.g.

v.setSpecificMessage(v.url, (issue) =>
    t("url", { received: issue.received, ns: "valibot" }),
  );

In zod-i18next you register error map like this

z.setErrorMap(
    makeZodI18nMap({
      // @ts-ignore
      t: t,
      ns: ["zod", "custom-zod"],
    }),
  );

and then when you're doing validation (like with rawCheck) you're passing the translation key and it uses the t function passed above, e.g.

function createSchema(
  intent: Intent | null,
  constraints?: {
    isEmailUnique?: (email: string) => Promise<boolean>;
  },
) {
  return z
    .object({
      email: EmailSchema.pipe(
        z.string().superRefine((val, ctx) => {

          return constraints.isEmailUnique(val).then((isUnique) => {
            if (!isUnique) {
              ctx.addIssue({
                code: "custom",
                params: {
                  i18n: "custom-zod:emailIsTaken",
                },
              });
            }
          });
        }),
      ),
    })
}

In this example it's not bound to any existing validator, which is really useful. I can just type namespace:translation-key in params.i18n key.

Is there a similar way in valibot now?

As of now, i call a global function like

setValibotTranslations(t);

which sets global messages for existing validators

export function setValibotTranslations(t: TFunction<["valibot"]>) {
  v.setSpecificMessage(v.bic, (issue) =>
    t("bic", { received: issue.received, ns: "valibot" }),
  );

  // Other translations...
}
jansedlon commented 3 months ago

I'm wondering if there's a way around that. From the type definition of IssueInfo$1 which is the interface i can pass to addIssue in valibot, there's no type property. This could be used to differentiate things. Like if {type: 'custom', message: 'namespace:key'} i could catch that in the setValibotTranslations and parse the message as a translation key and use the t function to translate it 🤔

Just throwing ideas around

Example:

function setValibotTranslations(t: TFunction<['valibot']>) {
  // Or set a string type? ↓↓↓↓↓
  v.setSpecificMessage(v.custom, (issue) =>
    {
      const isTranslationKeyRegex = /[a-zA-Z0-9_-]+:[a-zA-Z0-9_-]+$/;

      if (isTranslationKeyRegex.test(issue.message)) {
        return t(issue.message, { received: issue.received, ns: "valibot", defaultValue: issue.message });
      }

      return t("custom", { received: issue.received, ns: "valibot" })},
  );
}
fabian-hiller commented 3 months ago

i could catch that in the setValibotTranslations and parse the message as a translation key

This will not work because the message of rawCheck is more specific. So setSpecificMessage is ignored in this case.

You could try to create a function that takes a code as the first argument and returns another function that matches our ErrorMessage type. This way you could pass this function as the last optional argument to any schema or action to translate the error message. You can try it out in this playground.

import * as v from 'valibot';

type TranslationCode =
  | 'email:invalid'
  | 'email:format'
  | 'password:invalid'
  | 'password:length';

const translations: Record<string, Record<TranslationCode, string>> = {
  en: {
    'email:invalid': 'The email has the wrong data type',
    'email:format': 'The email is badly formatted',
    'password:invalid': 'The password has the wrong data type',
    'password:length': 'The password is too short',
  },
  de: {
    'email:invalid': 'Die E-Mail hat den falschen Datentyp',
    'email:format': 'Die E-Mail ist falsch formatiert',
    'password:invalid': 'Das Passwort hat den falschen Datentyp',
    'password:length': 'Das Passwort ist zu kurz',
  },
};

function t(code: TranslationCode): v.ErrorMessage<v.BaseIssue<unknown>> {
  return (issue) => translations[issue.lang || 'en']?.[code] ?? issue.message;
}

const Schema = v.object({
  email: v.pipe(v.string(t('email:invalid')), v.email(t('email:format'))),
  password: v.pipe(
    v.string(t('password:invalid')),
    v.minLength(8, t('password:length')),
  ),
});
ruiaraujo012 commented 3 months ago

@fabian-hiller Just to let you know that I've migrated today all the codebase of a project I'm working on to version v0.32.0, and it went well with the help of rawCheck. There are some places where I will change the checkItems when it's ready, but I was able to do a workaround with rawCheck for now.

Thank you.

fabian-hiller commented 3 months ago

checkItems is already implemented but not released as I am also working on a partialCheck action and some other stuff. I expect to release it today or in the next few days.

ruiaraujo012 commented 3 months ago

Np, the important one was rawCheck, checkItems is a plus for this codebase :)