[Suggestion] Instruct users to return z.NEVER in .superRefine() when providing a type predicate

[zetaraku]

As the validation result solely depends on whether ctx.addIssue() is called,

zod/README.md

Line 1891 in ad43854

You can add as many issues as you like. If `ctx.addIssue` is _not_ called during the execution of the function, validation passes.

and the return value is only used to satisfy the type predicate and is always thrown away,

zod/src/types.ts

Lines 3821 to 3822 in c9e4ed4

	// return value is ignored
	executeRefinement(inner.value);

to prevent users from accidentally forgetting to call ctx.addIssue() while returning a boolean value as described in the example,

zod/README.md

Lines 1931 to 1945 in ad43854

	.superRefine((arg, ctx): arg is { first: string; second: number } => {
	if (!arg) {
	ctx.addIssue({
	code: z.ZodIssueCode.custom, // customize your issue
	message: "object should exist",
	});
	return false;
	}
	return true;
	})
	// here, TS knows that arg is not null
	.refine((arg) => arg.first === "bob", "`first` is not `bob`!");
	```
	> ⚠️ You must **still** call `ctx.addIssue()` if using `superRefine()` with a type predicate function. Otherwise the refinement won't be validated.

instructing users to return z.NEVER instead of a boolean at the end (indicating the value is never used) just like the early return case when providing a type predicate may be a good idea.

  .superRefine((arg, ctx): arg is { first: string; second: number } => {
    if (!arg) {
      ctx.addIssue({
        code: z.ZodIssueCode.custom, // customize your issue
        message: "object should exist",
      });
    }

    return z.NEVER;  // this should satisfy the typing
  })

[maxArturo]

Hi @zetaraku I'm guessing you suggest this out of experience :) Shouldn't be a big lift...
I'll take a swing at it!

[maxArturo]

Ah - I spoke too soon. There is a quality-of-life situation at play here, where by doing what you suggest we would have to ask users to provide a type parameter in the call, which makes it a bit awkward since you're just trying to refine your validated output.

The reason being that if we expect the type signature to be refinement: (arg: Output, ctx: RefinementCtx) => z.NEVER then we can't pull out the type from the provided function. So I'd be hesitant to do that.

On the other hand! We can add some runtime-validation to make sure that ctx.addIssue() gets called at least once which in essence covers your use case. What do you think?

[zetaraku]

Hi, @maxArturo.

I knew that the return type is necessary for the inferred refined type so I didn't recommend users to declare their refinement function as refinement: (arg: Output, ctx: RefinementCtx) => z.NEVER but still like refinement: (arg: Output, ctx: RefinementCtx) => arg is something.

I only encourage users to return a z.NEVER in the refinement function. (See the last snippet above)

By the way, refinement: (arg: Output, ctx: RefinementCtx) => z.NEVER actually doesn't work because z.NEVER is a value of never type, not a type itself.

[maxArturo]

By the way, refinement: (arg: Output, ctx: RefinementCtx) => z.NEVER actually doesn't work because z.NEVER is a value of never type, not a type itself.

Yes understood! It was a copy paste in haste.

I only encourage users to return a z.NEVER in the refinement function. (See the last snippet above)

Sounds good, if your suggestion is to update the docs I think it is an improvement. I'd welcome the PR! Thanks!

[zetaraku]

I made the PR above to update the docs.
Please take a look at it and let me know if anything needs to be adjusted :)
I wonder if the test cases should be updated to return z.NEVER too. 🤔