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

How to control 2 fields equal or not #76

Closed
dukeofsoftware opened this issue Aug 7, 2023 · 52 comments
Closed

How to control 2 fields equal or not #76

dukeofsoftware opened this issue Aug 7, 2023 · 52 comments
Assignees
Labels
enhancement New feature or request priority This has priority

Comments

@dukeofsoftware
Copy link

in my case password and confirm password have to be equal but I can't find which method I have to use

@fabian-hiller fabian-hiller self-assigned this Aug 7, 2023
@fabian-hiller fabian-hiller added the question Further information is requested label Aug 7, 2023
@fabian-hiller
Copy link
Owner

For this you can use our pipeline feature: https://valibot.dev/guides/pipelines/

import { custom, email, minLength, object, string } from 'valibot';

const SignUpSchema = object(
  {
    email: string([
      minLength(1, 'Please enter your email.'),
      email('The email address is badly formatted.'),
    ]),
    password1: string([
      minLength(1, 'Please enter your password.'),
      minLength(8, 'You password must have 8 characters or more.'),
    ]),
    password2: string([minLength(1, 'Please repeat the password once.')]),
  },
  [
    custom(
      ({ password1, password2 }) => password1 === password2,
      'The passwords do not match.'
    ),
  ]
);

@dukeofsoftware
Copy link
Author

I tried this with react-hook-form but I don't get " the passwords do not match." error

@fabian-hiller
Copy link
Owner

fabian-hiller commented Aug 7, 2023

Have a look at this comment: #5 (comment)

I suspect that the problem has to do with the error being issued as an issue of the object and not the second password. Do you know how Zod solves this problem?

@dukeofsoftware
Copy link
Author

Thanks! I did this:

export const RegisterSchema = object(
  {
    email: string([
      minLength(1, "Please enter your email."),
      email("The email address is badly formatted."),
    ]),
    password: string([
      minLength(1, "Please enter your password."),
      minLength(8, "You password must have 8 characters or more."),
    ]),
    confirmPassword: string([
      minLength(1, "Please enter your password."),
      minLength(8, "You password must have 8 characters or more."),
    ]),
  },
  [
    (input) => {
      if (input.password !== input.confirmPassword) {
        throw new ValiError([
          {
            reason: "string",
            validation: "custom",
            origin: "value",
            message: "Passwords do not match.",
            input: input.confirmPassword,
            path: [
              {
                schema: "object",
                input: input,
                key: "confirmPassword",
                value: input.confirmPassword,
              },
            ],
          },
        ])
      }
      return input
    },
  ]
)

@fabian-hiller
Copy link
Owner

I will try to improve the DX for this use case before v1.

@Demivan
Copy link
Contributor

Demivan commented Aug 17, 2023

@fabian-hiller, Zod uses the same pipeline (refine in case of Zod) functionality. The problem with this approach is that the entire schema needs to be valid before the pipeline is executed.
This is fine when you need to check whether the entire data is valid, but is inconvenient for use in forms. Only when the user fills all fields correctly, will he see a "Passwords do not match." validation error.

I have added PR to implement partialRefine method to Zod (colinhacks/zod#2360), but it is not merged yet. Would be nice if valibot supported this, this is often overlooked by validation libraries.

@fabian-hiller
Copy link
Owner

Do you have any ideas on how we could integrate this into Valibot's API design?

@Demivan
Copy link
Contributor

Demivan commented Aug 17, 2023

Few ideas:

  • allow defining pipelines that are run on raw input. Input would have unknown type, so the developer would need to check everything they want to access.
  • allow pipelines on partially validated input (for object schemas), like Zod's partialRefine. This would work like a combination of pick plus a pipeline. The benefit of this is that the pipeline can have a strongly typed input.
  • Yup has ref to access sibling fields. Don't really like this solution - it is not typed and really breaks current Valibot's API design.

@Demivan
Copy link
Contributor

Demivan commented Aug 17, 2023

@fabian-hiller after some consideration, I think the cleanliest solution in terms of API could be something like this, inspired by Zod:

const schema = object({
  name: string(),
  password: string(),
  confirmPassword: string(),
}, [
  {
    type: 'pre', // Run before the validation
    transform: (data: unknown) => data.password === data.confirmPassword, // Input will be unknown
  },

  {
    type: 'post', // Run after validation (default)
    transform: (data: Typed) => data.password === data.confirmPassword, // Input will be typed

    // Allow specifying path and error message instead of throwing validation error
    path: ["confirmPassword"],
    message: "Both password and confirmation must match",
  },

  // Existing pipes are supported. Are the same as 'post' pipes
  custom((data) => data.password === data.confirmPassword),

  {
    // Object schema only
    type: 'partial', // Run after successfull validation of specified fields
    fields: ['password', 'confirmPassword'], // Array elements can be typed
    transform: (data: Pick<Typed, 'password' | 'confirmPassword'>) => data.password === data.confirmPassword, // Input will contain specified fields
  },
])

@fabian-hiller fabian-hiller reopened this Aug 17, 2023
@fabian-hiller
Copy link
Owner

Thanks a lot! I love to see code examples like yours. It helps to visualize the final implementation. Let's use this issue to gather more ideas. Then once I have the time to think about it in detail, we can move on to implementation.

@TFX0019
Copy link

TFX0019 commented Aug 27, 2023

export const RegisterSchema = object({
    body: object({
        email: string([minLength(1), email()]),
        password: string([
            minLength(1, 'Please enter your password.'),
            minLength(8, 'You password must have 8 characters or more.'),
        ]),
        confirmPass: string([
            minLength(1, 'Please repeat the password once.')
        ])
    }, [(input) => {
        if(input.password !== input.confirmPass) {
            return {
                issue: {
                    validation: 'custom',
                    message: 'The passwords do not match.',
                    input
                }
            }
        }
        return {output: input}
    }])
});

@irg1008
Copy link

irg1008 commented Sep 3, 2023

For me returning an "issue" object freezes the browser tab completely

@fabian-hiller
Copy link
Owner

Which Valibot version are you using? Can you share a reproduction on StackBlitz?

@fabian-hiller
Copy link
Owner

If you are using Valibot with a form library, it may also be because different versions that are not compatible are being used.

@irg1008
Copy link

irg1008 commented Sep 4, 2023

If you are using Valibot with a form library, it may also be because different versions that are not compatible are being used.

I think this may be the issue, I am using react hook forms with valibot resolver.
https://github.com/react-hook-form/resolvers#valibot

They are using valibot version:
"valibot": "^0.12.0"

@fabian-hiller
Copy link
Owner

I am not sure if that is the problem. Valibot is only registered as a peer dependency at React Form Hooks. Means that the version you specify in your package.json should be used.

Can you provide me a minimal reproduction e.g. via StackBlitz? Then I can investigate the problem.

@fabian-hiller
Copy link
Owner

@Demivan thanks again for your ideas. Maybe you can give feedback on this issue as well. Maybe we can avoid having to implement a pre-pipeline with a when function that allows to validate siblings. Or by allowing to access the parent or root input in general.

@AbePlays
Copy link

I was able to validate a field based on other field like this ↓

const TripInformationSchema = object(
  {
    departureCountry: string([minLength(2, 'Should have atleast 2 characters')]),
    destinationCountry: string([minLength(2, 'Should have atleast 2 characters')]),
    departureDate: string([
      isoDate(),
      custom(
        (date) => isAfter(new Date(date), new Date()) || isToday(new Date(date)),
        "The Departure Date must be on or after today's date."
      )
    ]),
    returnDate: string([isoDate()]),
    numberOfTravelers: enumType(['1', '2'], 'Invalid selection')
  },
  [
    (input) => {
      const isSuccess =
        isAfter(new Date(input.returnDate), new Date(input.departureDate)) ||
        isSameDay(new Date(input.departureDate), new Date(input.returnDate))

      if (!isSuccess) {
        return {
          issues: [
            {
              validation: 'custom',
              message: 'The Return Date must be on or after the Departure Date',
              input,
              path: [{ schema: 'object', input: input, key: 'returnDate', value: input.returnDate }]
            }
          ]
        }
      }
      return { output: input }
    }
  ]
)

This verifies the form's returnDate based on the departureDate.

@fabian-hiller
Copy link
Owner

Good point. Thanks for pointing that out. Is your concern that we need to run transform on a single entry despite issues in the pipeline so that the correct data type is passed to the object's pipeline? Below is an example:

import { custom, minLength, object, string, transform } from 'valibot';

const Schema = object(
  {
    key: transform(string([minLength(3)]), (input) => input.length),
  },
  [
    custom((input) => {
      // Is `input.key` a number or a string when `minLength` fails?
      console.log(input);
      return input.key >= 3;
    }),
  ]
);

If we perform transform anyway, we need to document it well.

I have investigated this with Zod in v3.22.4. Zod skips .transform but still performs .refine on the object, causing the data type of input.key to be incorrect. To me, Zod's approach feels like a bug.

import { z } from "zod";

const Schema = z
  .object({
    key: z
      .string()
      .min(3)
      .transform((input) => input.length),
  })
  .refine((input) => {
    // Type of `input.key` is `string` and therefore invalid if `min` fails!
    console.log(input);
    return input.key >= 3;
  });

@Demivan
Copy link
Contributor

Demivan commented Oct 23, 2023

Is your concern that we need to run transform on a single entry despite issues in the pipeline so that the correct data type is passed to the object's pipeline?

Yes, and it is not just transform too. Custom/function pipelines can transform field values. So the value could be different depending on where in the field pipeline error occurred.

The proposed solution sounds ok on paper, but there are too many edge cases in my opinion.

@fabian-hiller
Copy link
Owner

How would you proceed here?

@Demivan
Copy link
Contributor

Demivan commented Oct 23, 2023

I think, to compare two fields in an object schema, those two fields need to be valid first.

I propose in addition to custom function, add customPartial (probably named better 🙂) that accepts names of fields that you need to be valid. When those two fields are fully validated, then the pipeline will run.

So regular custom pipelines will still run when all object is valid, but customPartial pipelines will run as soon as all input fields are valid.

This is pretty much what I have proposed with partial pipelines, but without exposing implementation detail to consumers.

I can try sketching a pull request.

@fabian-hiller
Copy link
Owner

Good idea! Maybe we can build that into custom. Technically, this should work.

@Demivan
Copy link
Contributor

Demivan commented Oct 24, 2023

@fabian-hiller Added a proof of concept implementation of customPartial function

@skotenko
Copy link

skotenko commented Nov 3, 2023

For this you can use our pipeline feature: https://valibot.dev/guides/pipelines/

import { custom, email, minLength, object, string } from 'valibot';

const SignUpSchema = object(
  {
    email: string([
      minLength(1, 'Please enter your email.'),
      email('The email address is badly formatted.'),
    ]),
    password1: string([
      minLength(1, 'Please enter your password.'),
      minLength(8, 'You password must have 8 characters or more.'),
    ]),
    password2: string([minLength(1, 'Please repeat the password once.')]),
  },
  [
    custom(
      ({ password1, password2 }) => password1 === password2,
      'The passwords do not match.'
    ),
  ]
);

This approach is not working for me.
Is there a way to compare several fields?

@fabian-hiller
Copy link
Owner

@skotenko can you share your schema with us? Your problem is probably related to #76 (comment).

@skotenko
Copy link

skotenko commented Nov 4, 2023

@skotenko can you share your schema with us? Your problem is probably related to #76 (comment).

@fabian-hiller Yeah, thanks for the answer.
custom will be run if the form is successfully validated.

As I understood, there is no possibility to run custom earlier for now?
Thanks.

@fabian-hiller
Copy link
Owner

Yes, that is true. But hopefully I will find a final solution for this in the next few weeks.

@JortsEnjoyer0
Copy link

My opinion would be, in addition to @Demivan 's suggestion, would be to just have a 2nd argument passed into custom validators that have all the fields values so far. If you want to be able to specify "hey validate these other fields first before mine", ok, but that's added on top functionality. First and foremost let us read the values of other fields. I don't see that violating your API's paradigm.

@fabian-hiller
Copy link
Owner

@JortsEnjoyer0 the problem is that I think that there is no way to make it type safe because the custom function has no type information about its parents. Feel free to take a look at #260 and add your feedback there. I will try to take this into account when working on this feature in the next few days.

@fabian-hiller
Copy link
Owner

Previously, the pipelines of complex schemas were only executed if there were no issues. With the new version v0.22.0, pipelines are now always executed if the input matches the data type of the schema, even if issues have already occurred. In addition, the new method forward helps you to forward issues to a child.

In summary, it is now possible to compare two fields, for example password1 and password2, and in case of an error, forward the issue with the error message to password2:

import { custom, email, forward, minLength, object, string } from 'valibot';

const RegisterSchema = object(
  {
    email: string([
      minLength(1, 'Please enter your email.'),
      email('The email address is badly formatted.'),
    ]),
    password1: string([
      minLength(1, 'Please enter your password.'),
      minLength(8, 'Your password must have 8 characters or more.'),
    ]),
    password2: string(),
  },
  [
    forward(
      custom(
        (input) => input.password1 === input.password2,
        'The two passwords do not match.'
      ),
      ['password2']
    ),
  ]
);

I look forward to hearing your feedback on this solution.

@TepidDrink
Copy link

TepidDrink commented Dec 4, 2023

I am trying to implement this with a conditionally optional field, I can't seem to get it working

I have (within a schema object)

requiredRadio: enum_(RADIO_ENUM),
conditionalRadio: enum_(RADIO_ENUM),
...
forward(
  custom(
    (input) => input.requiredRadio === RADIO_ENUM.option1 && !input.conditionalRadio,
    'Please select a value'
  ),
  ['conditonalRadio']
),

I have tried changing conditionalRadio: enum_(RADIO_ENUM) to conditionalRadio: optional(enum_(RADIO_ENUM)), and a few other solutions but I have a couple issues:

  1. the error message is not propagated to the field
  2. the error only happens if all other fields are ok, I guess because its in the pipeline for after the initial validation, is there a way round that?

@fabian-hiller
Copy link
Owner

@TepidDrink can you provide me with a working schema so I can run and test it on my machine?

@TepidDrink
Copy link

TepidDrink commented Dec 5, 2023

import { parse, object, string, enum_, optional, forward, custom } from 'valibot';

enum RADIO_ENUM {
  'Yes',
  'No',
};

const DummySchema = object(
  {
    stringVal: string('Provide a value'),
    requiredRadio: enum_(RADIO_ENUM),
    conditionalRadio: optional(enum_(RADIO_ENUM)),
  },
  [
    forward(
      custom(
        (input) => input.requiredRadio === RADIO_ENUM.Yes && !input.conditionalRadio,
        'Please select a value'
      ),
      ['conditionalRadio']
    ),
  ]
);

parse(DummySchema, {requiredRadio: 'Yes'});

The concept here is that when requiredRadio is 'yes', the user needs to provide the conditionalRadio (I also threw in stringVal to show that the two validations are dealt with separately).

Ideally I'd like two error messages here: the one for stringVal and the one for conditionalRadio simultaneously, but instead it deals with stringVal only I guess because its in a pipeline for afterwards.

Removing/providing stringVal under current valibot implementation, conditionalRadio gets the generic 'Invalid Value' error message rather than 'Please select a value' which is the main issue.

@fabian-hiller
Copy link
Owner

@TepidDrink in your case, the pipeline of the object cannot be executed because stringVal is missing. Pipelines are typed. Therefore, they can only be executed if their input matches the defined data type.

With custom, note that the first argument is the requirement. The error message is only returned if the requirement is not met (the function returns false).

@Not-Saved
Copy link

Not-Saved commented Jan 18, 2024

I was trying to use custom and forward like the example you provided for password validation and I noticed that if I add a literal to the schema (ie. a privacy flag that must be true) the pipeline seem to only run if the literal is at true. I guess this is the intended behaviour as you mentioned above:

they can only be executed if their input matches the defined data type

What I'm trying to do is have the schema be valid only if literal is true but at the same time run the custom for password2. In the context of validating a form it's kinda awkward to not be able to show the password not matching error only after the user toggles the checkbox. Am I using the wrong tool here to accomplish this result?

The schema:

const RegisterSchema = object(
  {
    email: string([
      minLength(1, 'Please enter your email.'),
      email('The email address is badly formatted.'),
    ]),
    password1: string([
      minLength(1, 'Please enter your password.'),
      minLength(8, 'Your password must have 8 characters or more.'),
    ]),
    privacy: literal(true, 'Must be true'),
    password2: string(),
  },
  [
    forward(
      custom((input) => {
        //THIS ONLY RUNS IF PRIVACY VALUE IS TRUE
        return input.password1 === input.password2;
      }, 'The two passwords do not match.'),
      ['password2']
    ),
  ]
);

Stackblitz with a reproduction using a simple form in React.

I hope this was proper etiquette in reporting, first time I'm ever writing an issue on a project

@fabian-hiller
Copy link
Owner

fabian-hiller commented Jan 18, 2024

You could change the boolean literal to boolean([value(true)]) to solve this problem. This way the input matches the defined data type.

@fabian-hiller
Copy link
Owner

Update: Previously, check was not executed if the email field was untyped. This problem can now be solved with our new partialCheck action.

import * as v from 'valibot';

const RegisterSchema = v.pipe(
  v.object({
    email: v.pipe(
      v.string(),
      v.nonEmpty('Please enter your email.'),
      v.email('The email address is badly formatted.'),
    ),
    password1: v.pipe(
      v.string(),
      v.nonEmpty('Please enter your password.'),
      v.minLength(8, 'Your password must have 8 characters or more.'),
    ),
    password2: v.string(),
  }),
  v.forward(
    v.partialCheck(
      [['password1'], ['password2']],
      (input) => input.password1 === input.password2,
      'The two passwords do not match.',
    ),
    ['password2'],
  ),
);

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request priority This has priority
Projects
None yet
Development

No branches or pull requests