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

Calling isOptional triggers preprocess callback #1460

Closed
StefanTerdell opened this issue Oct 4, 2022 · 5 comments
Closed

Calling isOptional triggers preprocess callback #1460

StefanTerdell opened this issue Oct 4, 2022 · 5 comments

Comments

@StefanTerdell
Copy link
Contributor

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
Copy link

dearlordylord commented Oct 5, 2022

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

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
Copy link

Thank you for looking into this

@scotttrinh
Copy link
Collaborator

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
Copy link

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
Copy link

@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))
});

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants