colinhacks / zod

TypeScript-first schema validation with static type inference
https://zod.dev
MIT License
32.97k stars 1.14k forks source link

[Feature request] An option to always execute `refine`s even if parse failed, or workaround #2524

Open xsjcTony opened 1 year ago

xsjcTony commented 1 year ago

This is a simple form, with zod and react-hook-form, using @hookform/resolvers for integration. Reproduction link: https://stackblitz.com/edit/vitejs-vite-c6cz55?file=src%2FApp.tsx

This is an issue we've been talking about for a while... as in #479, #1394 and many other places... But I can't really find an ultimate solution for this.

So as part of my example, the validation schema is like this:

const schema = z
  .object({
    name: z.string().min(1),
    age: z.string()
      .transform((val) => (val ? +val : val))
      .pipe(z.number().min(18)),
    id_type: z.enum(['passport', 'driver_license']),
    passport: z.string().optional(),
    driver_license: z.string().optional(),
  })
  .refine(
    ({ id_type, passport }) => (id_type === 'passport' ? !!passport : true),
    { path: ['passport'], message: 'passport is required' }
  )
  .refine(
    ({ id_type, driver_license }) => id_type === 'driver_license' ? !!driver_license : true,
    { path: ['driver_license'], message: 'driver_license is required' }
  );

So in my reproduction, what I'm expecting is, once you click "Submit` button, all error messages should appear, including the "passport" one (by default if you don't select driver license).

But due to name and age are not entered at all (i.e. value is '' as the default value defined), hence it will halt and .refine() won't be executed at all.

I'm aware there's a concept called stopping or non-stopping issue (e.g. invalid_type compared to too_small), hence if I enter something like just a 1 in the age input box, the refines will be triggered. I also know it's due to type-safety purposes and I respect that...

But in the real-world use case, most likely it's not going to work like that. Apparently, I want to show all the error messages even if nothing is filled. This can be easily achieved by API like yup.when() but as @colinhacks clearly says it won't be part of zod, hence it's really a huge headache here.


One way to workaround is by z.intersection(), and define the id_type / passport and driver_license, as well as those .refine()s into another schema, but that's not really what I want. Although it works in particular cases, but it does break my expected typing system, e.g.

// When I can have something like this
type MyType = {
  foo: string;
  bar: string;
}
// Why do I have to do this?
type A = { foo: string };
type B = { bar: string };
type MyType = A & B;

And even worse, it's now a ZodIntersection instead of a ZodObject where I can't do something like .pick() / .omit() after that which is not good for extensibility.


Something to note:


I think this is the end of my explanation as everyone working on this kind of real-world form validation will run into this issue. Is there any chance we can have this feature (at least as an OPTION) implemented in zod or are there any better workarounds other than the awkward z.intersection()?

It's kinda a show-stopper for my migration from yup to zod... so I badly want a workaround without breaking the type (i.e. the final schema should still be a single object to align with my API)

xsjcTony commented 1 year ago

If using .merge() to merge ZodEffects could be possible, then lots of issues can be resolved. But I guess it's against zod's design principle... I'm not sure whether it's able to do so, but at least at the type level I can think about some overloads like this:

// Current implementation
merge<Incoming extends AnyZodObject, Augmentation extends Incoming["shape"]>(
  merging: Incoming
): ZodObject<
  objectUtil.extendShape<T, Augmentation>,
  Incoming["_def"]["unknownKeys"],
  Incoming["_def"]["catchall"]
>
// Proposed merging of ZodEffects
merge<
  Incoming extends AnyZodObject,
  Effect extends ZodEffects<ZodTypeAny, output<Incoming>>,
  Augmentation extends Incoming["shape"]
>(merging: Effect): ZodObject<
  objectUtil.extendShape<T, Augmentation>,
  Incoming["_def"]["unknownKeys"],
  Incoming["_def"]["catchall"]
>
rjmackay commented 1 year ago

I'm struggling to make something like this work too.

const schema = z.object({
  packages: z.array(z.object({ /* a bunch of props*/ })).optional(),
  packageCount: z.number().integer.optional(),
  serviceType: z.enum(['nextDay', 'sameDay'])
}).strict().refine(v => !(v.packages && v.packageCount)) // Only one-of packages or packages count can be define

schema.parse({ serviceType: 'junk' })

^ Will return an error for serviceType, but not the refine.

I've attempted to use an intersection, but that doesn't work well with strict(), and I can't use the refine() and merge() together either

aaronadamsCA commented 1 year ago

This was a helpful issue for me to read through, because it helped me better understand the design of Zod, and why this feature wouldn't make sense.

In short, if this were possible, then refine (and superRefine and transform) functions would need to be called with val: unknown, and there isn't really anything useful you could do with that.


At that point I think maybe this just becomes a documentation issue. I think "guide" materials could really help here, to show a variety of design choices that could be combined to solve these problems. For example:

const personSchema = z.object({
  name: z.string().min(1),
  age: z.coerce.number().min(18),
});

const idSchema = z
  .object({
    id_type: z.enum(["passport", "driver_license"]),
    passport: z.string().optional(),
    driver_license: z.string().optional(),
  })
  .superRefine((val, ctx) => {
    if (val.id_type === "passport" && val.passport == null) {
      ctx.addIssue({
        code: z.ZodIssueCode.custom,
        message: "Passport number is required",
      });
    }

    if (val.id_type === "driver_license" && val.driver_license == null) {
      ctx.addIssue({
        code: z.ZodIssueCode.custom,
        message: "Driver license number is required",
      });
    }
  });

const schema = z.intersection(personSchema, idSchema);

Yes, schema is a ZodIntersection instance, but personSchema and idSchema are still full ZodObject instances. I'm not sure there's anything you could do with a single ZodObject instance that you can't do with two of them - and if there is, that could be a more specific feature request.

If documentation like this were available, I think it would cut down on impossible requests like this, and help people compose better schemas. Thankfully I think I learned enough from the above to improve some of my own schemas as a result.

xsjcTony commented 1 year ago

@aaronadamsCA Thanks for the reply, but for your intersection implementation, a huge con is like your said, it's not a ZodObject anymore, which means all those ZodObject methods cannot be used anymore.

Including useful .keyof() / .pick() / .omit(), etc., although it's working in TypeScript (when mirrored use-case to types), but not in zod (runtime) https://zod.dev/?id=objects

xsjcTony commented 1 year ago

bump

maurer2 commented 1 year ago

Hello, I noticed that this behaviour is highlighted as a known issue in the official documentation of the popular Vue3 validation library vee-validate: https://vee-validate.logaretm.com/v4/integrations/zod-schema-validation/

Cheers

Brunnn commented 10 months ago

This issue is giving me a lot of headaches, and for many other people, I know that zod is a typescript first project, but would it hurt that much to make this type of "custom developer code" be executed with a unknown type? The deal with refinements in zod is to put custom validation logic in your schemas, so I can't see why this feature should have a requirement of "only executes if zod's built in parsing passes"...

I dug up a little bit in the source code and experimented a little, i've came up with a scaffold of a solution, I've tested and it works:

type.ts

//Original
 superRefine(
    refinement: (ctx: RefinementCtx) => unknown | Promise<unknown>
  ): ZodEffects<this, Output, Input> {
    return this._refinement(refinement);
  }

//Modified
//Options could be a ZodEffectOptions parameter...
 superRefine<T extends boolean = false>(
    refinement: (arg: T extends false ? Output : unknown, ctx: RefinementCtx) => unknown | Promise<unknown>,
    options?: { ignoreParsing: T }
  ): ZodEffects<this, Output, Input> {
    return this._refinement(refinement, options);
  }
  //original
  _refinement(
    refinement: RefinementEffect<Output>["refinement"],
  ): ZodEffects<this, Output, Input> {
    return new ZodEffects({
      schema: this,
      typeName: ZodFirstPartyTypeKind.ZodEffects,
      effect: { type: "refinement", refinement },
    });
  }

  //modified
  _refinement(
    refinement: RefinementEffect<Output>["refinement"],
    options?: { ignoreParsing: boolean }
  ): ZodEffects<this, Output, Input> {
    return new ZodEffects({
      schema: this,
      typeName: ZodFirstPartyTypeKind.ZodEffects,
      effect: { type: "refinement", refinement },
    }, options);
  }

and then when executing the refinement effect, we just check if the flag is set to ignore the parsing, and just call the refinement with ctx.data instead of the parsed value. The good thing about this approach, is that the values will only be of the type unknown if the flag is set to true (leaving the default behavior as it is today)

Would something like that be considered in the project?

Svish commented 9 months ago

refine, superRefine and transform should stay the way they are. They make zod very reliable and trustworthy both with regards to the types it produces and the validation and parsing it does.

If there should be any changes to zod at all to allow for these more complex form-validation issues, how about just adding a new function instead, that gets unknown as parameter, is called regardless, has no effect on inferred types, and just allows you to add errors to context, like superRefine and transform?

akomm commented 7 months ago

If .refine would be called with invalid input, then the only value type you could see in refine would be unknown, because anything is possible. That would render the refine basically useless.

I have a specific approach that also helps me solve this sort of issue (and much more). When handling user input, I do not try to map it directly into a model, but instead create the Input type using the model definition. Due to unsoundness and other issues, I do not use omit and pick, but instead use .shape:

/* Model.ts */

const Model = z.object({
  id: z.string(),
  foo: z.string(),
  // we just assume this has to be in this form in the Model
  validFrom: z.string().datetime(),
  validThru: z.string().datetime()
})

/* createModel.ts */

// I have freedom to shape the input validation as I see fit and only pick what I need to parse from user.
// errors outside of user submission are *app errors* not *user errors*
const UserInput = z.object({
  foo: Model.shape.foo,
  // .refine will run once the *nested* z.object is valid,
  // because refine's dependency is just the from-thru object now!
  // It does not depend on the entire UserInput anymore.
  valid: z.object({
    from: Model.shape.validFrom,
    thru: Model.shape.validThru
  }).refine(({from, thru}) => from < thru, "valid from > valid thru")
  // Extra: you could also extract *valid* for example as DatetimeRange
})

const result = UserInput.safeParse(data)
if (!result.success) {
  // the error shape is type-safe to be rendered in front end UI, if you have server->client type inferrence!
  // your form structure is not tied to the model structure!
  return {errors: result.format()} 
}

const {foo, valid} = result.data
const model: Model = { // no need to type :Model, but it makes it exhaustive when you *add* or *change* fields
  id: generateId(),
  foo,
  validFrom: valid.from,
  validThru: valid.thru
}

Added example for safeParse in remix using formData:

const result = UserInput.safeParse({
  foo: form.get('foo'),
  valid: {
    // the names are no *magic*, its literally just name="valid.from", you can name it simple and flat, not object processing needed!
    from: form.get('valid.from'),
    thru: form.get('valid.thru')
  }
})

As you can see, this approach solved a lot of issues, including the refinement of compounds.

jimmi-joensson commented 4 weeks ago

I came up with this helper that treats the data as if its a zod any but then returns the type that was passed into it. Not sure if theres any differences between z.any and z.*, but all I need from this is refine before using the product as a child validation unit or for final validation. My use-case for this is validating forms with react hook forms, where the validator gets run on field change and the data partially fills the schema.

export function zodAlwaysRefine<T extends z.ZodTypeAny>(zodType: T) {
  return z.any().superRefine(async (value, ctx) => {
    const res = await zodType.safeParseAsync(value);

    if (res.success === false)
      for (const issue of res.error.issues) {
        ctx.addIssue(issue);
      }
  }) as unknown as T;
}

// example:

zodAlwaysRefine(
  z.array(
    z.object({
      key: z.string(),
      name: z.string(),
    }),
  ),
).superRefine((value, ctx) => {
  // this should get run even if any `name`s are missing, and the zod type is untouched for further chaining
  // even though certain properties could be undefined (so its technically not typesafe, but hopefully logic in the refine is small)
  //
  // ex. *find duplicates and add errors*
  // ...
});

However, it would be nice to have some sort of flag that you can pass into any zod unit that validates regardless of the intrinsic validation result.

I just wanted to share @mtjlittle's solution here and hopefully we will see it implemented in the future. You can see the original post here.