refine() function gets called even when parser already failed

[ctollerud]

Using version 3.20.6

In certain scenarios, refine gets called even when the parser has already failed. It seems like including a z.custom() can trigger this.

What's worse is that the type flowing into the refine's function (x) is different than what typescript thinks it is

Here's a minimal example:

const example1 = z
	.custom<number>(x => typeof x === 'number')
	.transform(x => String(x))
	.refine(x => {
		console.log(typeof x); // prints 'Object'
		console.log("I get called even though I shouldn't!!!");
		return true;
	})
	.safeParse({}); //will fail because it is not a number

console.log(example1.success); // false (like it should be)

And here's a more complex example with the same issue

const example2 = z
	.union([z.number(), z.string(), z.custom<number>(x => typeof x === 'number')])
	.refine(() => true)
	.pipe(z.any())
	.or(z.date())
	.pipe(z.any())
	.refine(x => {
		console.log("I get called even though I shouldn't!!!!!!!!!");
		return true;
	})
	.safeParse(true); // will fail immediately because it's a bool

As a work-around, using .transform(x,ctx)=>...) approach to validate seems to work.

[JacobWeisenburger]

I have confirmed that this works as stated by @ctollerud

[ctollerud]

In case it helps, here's a more general repeatable pattern: It looks any failure that's caused by refine will not prevent any future refines from being called.

const result = z
	.unknown() //let everything through
	.refine(() => false) // force failure for everything here
	.refine(x => {
		console.log('I should not get called');
		return x;
	})
	.refine(x => {
		console.log('I should also not get called');
		return x;
	})
	.safeParse('asdf');
console.log(result.success); // fails (correctly)

It also looks like failure inside of z.string().regex(...) can trigger this behavior: https://stackoverflow.com/questions/74806649/zod-refine-arguments-provide-the-unvalidated-raw-data

Here's a minimal repro of that as well:

const result2 = z
	.string()
	.regex(/123/)
	.refine(x => {
		console.log('I should not be called');
		return x;
	})
	.safeParse('234');

console.log(result2.success); // fails (correctly)

[colinhacks]

This is fixed in Zod 3.21

[JacobWeisenburger]

This is fixed in Zod 3.21

I have confirmed this too

[JacobWeisenburger]

This is not working correctly again.

const result = z
    .unknown() //let everything through
    .refine( () => false ) // force failure for everything here
    .refine( x => {
        console.log( 'I should not get called' )
        return x
    } )
    .refine( x => {
        console.log( 'I should also not get called' )
        return x
    } )
    .safeParse( 'asdf' )
console.log( result.success ) // fails (correctly)

const result2 = z
    .string()
    .regex( /123/ )
    .refine( x => {
        console.log( 'I should not be called' )
        return x
    } )
    .safeParse( '234' )

console.log( result2.success ) // fails (correctly)

[Darkhogg]

Are this and #2192 the same issue? they seem to be...

[JacobWeisenburger]

they are very similar

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[ctollerud]

Please don’t close! This is definitely a bug that should be addressed.

I can take a stab at it if no one else gets to it, but note there’s also a pr pending review already out there.

On Mon, Jun 12, 2023 at 6:24 PM stale[bot] @.***> wrote:

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

— Reply to this email directly, view it on GitHub https://github.com/colinhacks/zod/issues/2113#issuecomment-1587752631, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABPTF3IRD6KLTH4U2ELOCQ3XK5GDFANCNFSM6AAAAAAVJ3QS2Y . You are receiving this because you were mentioned.Message ID: @.***>

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[akomm]

Can the stale bot not be configured to not mark issues that are bug-confirmed?

I think there was some reasoning behind making .min and similar built-in refines non-fatal. I think it was that when type is parsed successfully, e. G. here number, that its all needed to further refine it. But this assumption is wrong. You might have refinements based on assumption about the previously validated data. For example that something is not zero, or that something is a sortable, non-empty string, example:

const dateIsoRegex = /^\d{4}-\d{2}-\d{2}$/
const dateIso = z
    .string()
    .refine(s => s.match(dateIsoRegex) && !Number.isNaN(Date.parse(s)), "Invalid Date")
    .brand("DateIso")

const dateIsoRange = z
    .object({from: dateIso, thru: dateIso})
    .refine(({from, thru}) => from < thru, "from > thru")
    .brand("DateIsoRange")

Besides producing "Invalid Date" error, this one will randomly, depending on the input, also tell that from is greater than thru.

Current workaround:

const dateIso = z
  .string()
  .superRefine((v, ctx) => {
    if (!v.match(dateIsoRegex) || Number.isNaN(Date.parse(v))) {
      ctx.addIssue({
        code: z.ZodIssueCode.custom,
        fatal: true,
        message: "Invalid Date",
      })
    }
  })