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

zod.preprocess v3.22.2 issues are ignored in case of another error in the model #2677

Open
azhiv opened this issue Aug 22, 2023 · 6 comments
Labels
bug-confirmed Bug report that is confirmed

Comments

@azhiv
Copy link
Contributor

azhiv commented Aug 22, 2023

Another issue we found in v3.22.2 is that the schema inside preprocess either isn't executed or doesn't add issues to the context in case the model being parsed contains an error in another field (nonNullableObject below):

import * as zod from 'zod';

const wrapToArray = <T extends zod.ZodTypeAny>(elementSchema: T) =>
  zod.preprocess((val) => [val], zod.array(elementSchema));

const singleItemSchema = zod.object({
  quantity: zod.number(),
});

const itemsStepSchema = zod.object({
  nonNullableObject: zod.object({}),
  items: wrapToArray(singleItemSchema), // swapping this line with the one below results in correct behaviour (2 issues instead of 1)
  // items: singleItemSchema,       
});

const res = itemsStepSchema.safeParse({
  nonNullableObject: null,
  items: {},
});

//@ts-ignore
console.log(res.error.issues.length);  // 3.21 shows 2 errors, 3.22 - only 1
console.log(res);

An error in nonNullableObject hides the preprocess errors. Also, if I remove the preprocess call completely the behaviour becomes correct. This doesn't look right as preprocess stops errors from being populated.
3.21.4:
Screenshot 2023-08-21 at 19 06 17

3.22.2:
Screenshot 2023-08-21 at 19 05 12

@cmd-johnson
Copy link

cmd-johnson commented Aug 23, 2023

I just stumbled over this one as well. A simple test case to reproduce:

const result = z
  .array(z.preprocess((v) => v, z.number()))
  .safeParse(['a', 'b'])

expect((result as z.SafeParseError<unknown>).error.errors).toStrictEqual([
  {
    code: 'invalid_type',
    expected: 'number',
    received: 'string',
    path: [0],
    message: 'Expected number, received string',
  },
  {
    code: 'invalid_type',
    expected: 'number',
    received: 'string',
    path: [1],
    message: 'Expected number, received string',
  },
])

This test fails because safeParse only returns the first error. Removing the z.preprocess (which shouldn't do anything in this case anyway) makes the test pass.

I can also confirm that the latest version where this worked as expected was v3.21.4 and only versions v3.22.0 and later are affected.

@maorun
Copy link

maorun commented Sep 22, 2023

fix with #2719 ?

@JacobWeisenburger
Copy link
Collaborator

This seems to be a bug with preprocess. Perhaps using transform and pipe will work for you instead?

const wrapToArray = <T extends z.ZodTypeAny> ( schema: T ) =>
    z.any().transform( x => [ x ] ).pipe( schema.array() )

const schema = z.object( {
    foo: z.number(),
    bar: wrapToArray( z.object( { qux: z.number() } ) ),
} )

const result = schema.safeParse( { foo: 'invalid', bar: 'invalid' } )
!result.success && console.log( result.error.issues )
// [
//     {
//         code: "invalid_type",
//         expected: "number",
//         received: "string",
//         path: [ "foo" ],
//         message: "Expected number, received string"
//     }, {
//         code: "invalid_type",
//         expected: "object",
//         received: "string",
//         path: [ "bar", 0 ],
//         message: "Expected object, received string"
//     }
// ]

If you found my answer satisfactory, please consider supporting me. Even a small amount is greatly appreciated. Thanks friend! 🙏
https://github.com/sponsors/JacobWeisenburger

@JacobWeisenburger JacobWeisenburger added the bug-confirmed Bug report that is confirmed label Sep 23, 2023
@thiagomeireless
Copy link

Hey @JacobWeisenburger, your solution works for small projects, but we currently have multiple projects/services with lots of schemas using preprocess and changing them all is not a good option.

Would the fix proposed in #2719 be good enough to merge and release a quick fix? Multiple people are trying to update to latest version of Zod to fix the Email Regex ReDoS vulnerability but this preprocess bug introduced in https://github.com/colinhacks/zod/releases/tag/v3.22.0 is a breaking change...

@kocyigityunus
Copy link

#2719 seems good enough.

@andreyfel
Copy link

Seems to be fixed in 3.23.0 with #2912

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug-confirmed Bug report that is confirmed
Projects
None yet
Development

No branches or pull requests

7 participants