Chaining refinements which narrow down types gives them inconsistent runtime/static types

[DanielBreiner]

Validation continues after an unsuccessful refine(), which gives it an incorrect return type and can make subsequent effects unexpectedly throw.

Code:

import { z } from 'zod';

const isNotNull = <T>(val: T | null): val is T => val !== null;

const userSchema = z
  .object({
    type: z.literal('Staff'),
    age: z.number(),
  })
  .nullable()
  // Type guard, I would expect validation to halt here if value is null
  .refine(isNotNull, { message: 'Choose a user type.', path: ['type'] })
  // User is infered as non-null
  .superRefine((user, ctx) => {
    // This should always return false (even according to TS), but it doesn't
    console.log(`superRefine: Is user null: ${user === null}`);
    // If user is null, we get an exception here
    if (user.age < 18) {
      ctx.addIssue({
        code: z.ZodIssueCode.custom,
      });
    }
  });

const goodResult = userSchema.safeParse({
  type: 'Staff',
  age: 20,
});
console.log({
  success: goodResult.success,
  output: goodResult.success ? goodResult.data : goodResult.error.issues,
});
const issueResult = userSchema.safeParse({
  type: 'Staff',
  age: 15,
});
console.log({
  success: issueResult.success,
  output: issueResult.success ? issueResult.data : issueResult.error.issues,
});
const bugResult = userSchema.safeParse(null); // Exception
console.log({
  success: bugResult.success,
  output: bugResult.success ? bugResult.data : bugResult.error.issues,
});

Output:

superRefine: Is user null: false
{ success: true, output: { type: 'Staff', age: 20 } }
superRefine: Is user null: false
{
  success: false,
  output: [ { code: 'custom', path: [], message: 'Invalid input' } ]
}
superRefine: Is user null: true
[TypeError: Cannot read properties of null (reading 'age')]

Expected output (last line):

{
  success: false,
  output: [ { code: 'custom', message: 'Choose a user type.', path: [Array] } ]
}

Yes, there is an easy workaround by adding an if (user !== null) return; (which I have used for now). But the exception comes out of nowhere and the runtime type is different from the static type (TS tells us user can't be null) which can be very confusing and make the issue hard to find.

[maxArturo]

Whoa, for a second I was also pretty confused about this @DanielBreiner. There's a feature that will let you abort early but you will have to use superRefine to gain access to it. In your example, it will actually let you know that further refinement on user is possibly null:

  const userSchema = z
    .object({
      type: z.literal("Staff"),
      age: z.number(),
    })
    .nullable()
    .superRefine((user, ctx) => {
    // Validation will halt here
      if (!isNotNull(user)) {
        ctx.addIssue({
          code: z.ZodIssueCode.custom,
          message: "user shouldn't be null",
          fatal: true,
        });
      }
    })
    .superRefine((user, ctx) => {
      console.log(`superRefine: Is user null: ${user === null}`);
      // If user is null, we get an exception here
      if (user.age < 18) { // User is infered as possibly null here
        ctx.addIssue({
          code: z.ZodIssueCode.custom,
        });
      }
    });

[DanielBreiner]

Yes, refine infers the narrowed type, but superRefine has no way of doing so.

refine<RefinedOutput extends Output>(
  check: (arg: Output) => arg is RefinedOutput,
  message?: /* ... */
): ZodEffects<this, RefinedOutput, RefinedOutput>;

superRefine: (
  refinement: RefinementEffect<Output>["refinement"]
) => ZodEffects<this, Output, Input>;

[maxArturo]

@DanielBreiner you're right! The type parameters do not extend output on superRefine, so it wouldn't be able to narrow down the type. We'd need to make a change so that the function signature, a return value or a type parameter would need to feed into the chained superRefine type. Makes sense to me that the narrowed types should reflect a concrete type output.

Actually re-reading your issue, do you mean you'd like refine() to have a abort-early functionality so it stops validation? And/or that superRefine() narrow down types? Both make sense to me, just making sure what the issue ask is.

[DanielBreiner]

It seems to me adding an option to abort on refine failure is the way to go in this case, but the docs should mention the possible runtime/static type mismatch if you do not abort (which always happens now).

I do also think superRefine should have a way of narrowing down types. I could create a separate issue for that.

[DanielBreiner]

Alright, I did some cleaning up and split this issue into three: #1602 - Add a way for .superRefine() to narrow down the output type #1603 - Add an option to .refine() to abort early This issue - Discussion/possible ways of fixing the inconsistent typing when chaining refinements.

To me, the most apparent fixes to this issue are:

1. Implement #1603 and add a warning to the docs stating that chaining refinements will cause this type inconsistency.
2. Give effects another type parameter which has the non-narrowed type (output type stays narrowed).

1 seems easier but leads to incorrect types. I do not know how difficult 2 would be or if it is even doable.

Would love to hear other opinions on this.

[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.