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

Feat/add guard #3862

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Feat/add guard #3862

wants to merge 4 commits into from

Conversation

oberbeck
Copy link

@oberbeck oberbeck commented Nov 19, 2024

Proposal: reintroducing type guard

Summary

This proposal seeks to reintroduce a type guard functionality to Zod, with a revised and safer implementation. AFAIK the removal of the legacy check method was prompted by the adoption of transformers, due to its potential disconnect between Input and Output types. This new implementation, however, reintroduces a type guard with a strict condition: it is only available when the Output type extends the Input type, ensuring the guard is actually correct.

Why is this necessary?

Zod is a TypeScript-first schema validation library with robust static type inference. While the safeParse method in Zod provides runtime validation, it does not allow TypeScript to infer the type of data in the broader scope after validation.

This new guard solves that limitation by ensuring that TypeScript can infer the type of the validated data at compile-time, making the development experience smoother and safer.

Proposed Implementation

zod/deno/lib/types.ts

Lines 521 to 526 in 47f04a9

guard<T extends Output>(
this: ZodType<T, any, T>,
data: unknown
): data is Output {
return this.safeParse(data).success;
}

Naming

As it has already been discussed in various issues, I also suggest to avoid overlap with the legacy check method and therefore use a fresh name for this implementation. My proposal uses guard, which I think clearly conveys its purpose. An alternative name could be is, ...?

potentially resolves

Update:

I just realised the coerce options do not have an impact on the Input making this solution potentially incorrect.
I changed the PR to DRAFT and am looking into this now. #2413 (comment)

Update:

AFAICT coercion should influence the Input type as well. To address this, I’ve adjusted the coercion implementations to ensure they impact Input, making the guards also work in those cases.

TODOs

  • check for more occurrences where Output extends Input incorrectly which would render this guard incorrect

Copy link

netlify bot commented Nov 19, 2024

Deploy Preview for guileless-rolypoly-866f8a ready!

Built without sensitive environment variables

Name Link
🔨 Latest commit 1afd010
🔍 Latest deploy log https://app.netlify.com/sites/guileless-rolypoly-866f8a/deploys/673da80ec1331700081071da
😎 Deploy Preview https://deploy-preview-3862--guileless-rolypoly-866f8a.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@oberbeck oberbeck marked this pull request as draft November 19, 2024 18:32
@oberbeck oberbeck marked this pull request as ready for review November 19, 2024 20:12
@oberbeck
Copy link
Author

Considering the Output extends Input condition, in cases where Output *does not* extend Input, we can still validate if data matches the Input type using a representation like the following:

  guard(this: ZodType<Output>, data: unknown): data is Output;
  guard(this: ZodType<Output, ZodTypeDef, Input>, data: unknown): data is Input;
  guard(data: unknown): boolean {
    return this.safeParse(data).success;
  }

Copy link

@ahrjarrett ahrjarrett left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool PR! I've done something similar in the past, so it's cool to see others have a similar use case.

I have a suggestion for this PR, and a recommendation that I thought you might find interesting.

Suggestion:

It might be worth exploring a solution that doesn't "crowd" existing behavior for users that don't care about this feature.

For example, adding a type parameter to all string schemas means all users will now see that type parameter everywhere they use z.string in their schemas, which adds noise. It wouldn't break anything, but it does get pretty noisy, and could hurt the DX overall.

Recommendation

If this is something you're needing, here's a pattern you can use to extend any library's behavior. YMMV ofc, and I've found libraries that ship combinators (like zod) are especially good fits for this approach.

You'd probably want to use a class so you can extend the existing behavior, but in general I've found this approach to be more flexible than it might appear at first glance.

https://tsplay.dev/W4DBBW

Edit: fixed a bug in the playground

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.

Runtypes-like .guard()
2 participants