Calling `isOptional` triggers preprocess callback

[StefanTerdell]

Calling isOptional triggers preprocess callback.

Related to StefanTerdell/zod-to-json-schema#23.

Here's a failing test case:

test("calling isOptional should not trigger preprocess function", () => {
  let wasCalled = false;

  const pre = z.preprocess(() => {
    wasCalled = true;
  }, z.string());

  pre.isOptional();

  expect(wasCalled).toBe(false);
});

[dearlordylord]

Maybe it's not an easy fix - looking thru code

zod/src/types.ts

Line 439 in 4709160

return this.safeParse(undefined).success;

it seems that actually running a parser is crucial for this method

If it's not possible to change, or if it's planned to work like this, this would be a documentation issue; in such a case it's needed to mention that .preprocess cb must be a pure function in docs

[dearlordylord]

Thank you for looking into this

[scotttrinh]

I believe this is "working as intended" and one of the gotchas for the preprocess feature. Admittedly, preprocess is not my favorite feature, but it does have its use cases and proponents. You can think of preprocess as something that always runs when you interact with the schema at runtime, even if in some cases it doesn't I don't think we're likely to guarantee that any given method (or even property lookup!) won't run the preprocess first: sometimes we just want to safeParse the schema! 😅

Going to close for now, but feel free to keep the conversation alive if you have any further thoughts.

[dearlordylord]

I currently get by with transform, with latest zod version it's a bit more useful for this case. My case being a string ${literal}${SEPARATOR}${brandedId} to be parsed and accessible as {type: ...literal..., id: ...brandedId...} and some similar shenanigans

[ciscoheat]

@StefanTerdell is this the same problem as when using zod-to-json-schema with a schema such as this?

const schema = z.object({
  name: z.string().min(1),
  email: z
    .string()
    .optional()
    .refine(async () => Promise.resolve(true))
});

[StefanTerdell]

@ciscoheat I might need to adjust my notification settings 😓 But yes, looks like it. I will look into it again soon.