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

Fix ctx.addIssue in transform to work correctly with parseAsync #1129

Merged
merged 2 commits into from May 12, 2022

Conversation

bentefay
Copy link
Contributor

@bentefay bentefay commented May 11, 2022

A recent PR added ctx.addIssue to transform: #1056

This is fantastic!

There appears to be a small bug - calling ctx.addIssue in transform does not work correctly when using parseAsync. This results in a confusing behaviour where parseAsync and parse return different results.

Reproduction

z
  .string()
  .transform((val, ctx) => {
    const parsed = parseInt(val);
    if (isNaN(parsed)) {
      ctx.addIssue({
        code: z.ZodIssueCode.custom,
        message: "Not a number",
      });
    }
    return parsed;
  })
  .parse("Not a number");

Expected: Zod error
Actual: Success

I've added a test to confirm it is now working correctly. I also took the chance to update the README to reflect this new feature.

@bentefay bentefay force-pushed the fix-transform-ctx-parseasync branch from 2f68ac3 to 49081fd Compare May 11, 2022 11:17
@bentefay bentefay force-pushed the fix-transform-ctx-parseasync branch from 49081fd to 71a33c5 Compare May 11, 2022 11:42
@bentefay bentefay force-pushed the fix-transform-ctx-parseasync branch from 71a33c5 to e8af058 Compare May 11, 2022 11:44
@colinhacks
Copy link
Owner

Looks solid, and good catch!

@colinhacks colinhacks merged commit ab8a371 into colinhacks:master May 12, 2022
MrAwesome pushed a commit to MrAwesome/zod that referenced this pull request May 20, 2022
…nhacks#1129)

* Fix transform ctx.addIssue to work with parseAsync

* Add documentation about transform taking ctx
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

Successfully merging this pull request may close these issues.

None yet

2 participants