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

Override safeParse and parse behavior at a given key #3448

Open
EllAchE opened this issue Apr 27, 2024 · 9 comments
Open

Override safeParse and parse behavior at a given key #3448

EllAchE opened this issue Apr 27, 2024 · 9 comments

Comments

@EllAchE
Copy link

EllAchE commented Apr 27, 2024

I'm looking to implement a Zod schema that supports "hard" and "soft" type definitions in the same schema with little boilerplate. What I mean by that is combining the behaviors of parse and safeParse in the same schema.

For example, given a schema

export const simpleSchema = z.object({
  errorIfNotString: z.string(),
  warnIfNotString: z.string(),
});

If the parsed object doesn't have a string value for errorIfNotString I want to throw an error. If warnIfNotString is undefined, number type etc. I want to follow safeParse behavior and return an error message, but not throw. I understand this would be possible right now, I could define two separate schemas and call safeParse for one & parse with the other. However this can get complex for large objects, so I'd prefer an API that is something like the below:

export const simpleSchema = z.object({
  errorIfNotString: z.string().assert(),
  warnIfNotString: z.string().warn(),
});

The expected behavior would be an override of the called parse method. I.e. if I call parse() on simpleSchema, zod should still not throw an error in the event of mismatch on warnIfNotString. And if I call safeParse zod should still error if errorIfNotString ends up being a mismatch.

Happy to look into this and come back with a PR too, just wanted to get the discussion started!

Some quick things to note:

  • the response type likely should always match safe parse if using this approach. It might be simplest to add a third .parse type, i.e. .multiParse that behaves like parse +
@EllAchE
Copy link
Author

EllAchE commented Apr 28, 2024

Updating on this - realizing what I want is aversion of safeParse that returns its best attempt at parsing the schema alongside the error. So even if there is a failure in a given subschema I'd still want to return as much data as possible.

@m10rten
Copy link
Contributor

m10rten commented Apr 30, 2024

Just making the fields optional and or nullable is not enough?

Combined with a .passtrhough should be plenty.

@EllAchE
Copy link
Author

EllAchE commented May 1, 2024

Just making the fields optional and or nullable is not enough?

Combined with a .passtrhough should be plenty.

@m10rten thanks for the response! I'm happy to work with existing apis if they achieve the goal, but as I understand my use case isn't supported (without adding custom transports/logic). I think this down tosafeParse not returning data in the case of schema mismatch. i.e. from https://zod.dev/?id=safeparse

.safeParse(data:unknown): { success: true; data: T; } | { success: false; error: ZodError; }

As an example - I have some fields that are absolutely essential, and some that are non essential. I want to be warned when non essential fields are not present (or are but not in expected type) but I still want to parse the data in the response type. Putting a trivial example here. If my schema were

const simpleSchema = z.object({
  errorIfNotString: z.string(),
  warnIfNotString: z.string(),
});

And I was parsing two objects, like below

// this should throw
{
  errorIfNotString: 999; // number type
  warnIfNotString: "valid";
}

// this should warn
{
  errorIfNotString: "999"; 
  warnIfNotString: 999; // number type
}

The desired behavior is an error thrown in the first case, while in second I would expect to get {success: false, error: ZodError, data: any}.

In case 2 I'd expect "data" to be

{
  errorIfNotString: "999"; 
  warnIfNotString: 999; 
}

Can I achieve this using the library as it is right now? Is it possible (without lots of boilerplate) in large schemas?

@m10rten
Copy link
Contributor

m10rten commented May 1, 2024

I suppose you could make a warn function that takes a zod schema and never errors, but that destroys your request, so the way I see it, this could be useful but should always be extended behaviour and not default.

@EllAchE
Copy link
Author

EllAchE commented May 1, 2024

I suppose you could make a warn function that takes a zod schema and never errors, but that destroys your request, so the way I see it, this could be useful but should always be extended behaviour and not default.

Thanks for the response! Not sure I understand what you mean by destroys your request. If I understand correctly I don't believe that's what I want; I'd still want to error when "essential data" is not present. Ideally I'd want to be able to parse with a single api (i.e. multiParse, or a better name) and define whether a schema throws or warns inline if validation fails.

And to be explicit about my suggestion - it would be an extension, not default. I'd want to add .warn() and .assert() (or other names) to schemas to modify behavior, when parse and safeParse are called without these additions I'd expect the current behavior. Unfortunately I don't think it would be possibly to make .warn() useful with parse without modifying the return type, which would be a breaking change. This is why I suggested the concept of "multiparse", to preserve backwards compatibility.

Also, I want to draw a distinction between 2 related but distinct use cases I'd want to support. The labels I'd put on each are

  • Return data alongside safeParse failures
  • Override safeParse and parse dynamically

I think a new discussion/issue is warranted for the first item as it's a separate ask. It's also the api I care more about. Can continue discussion for the second item in this thread as it is appropriately named.

@colinhacks
Copy link
Owner

Zod would benefit from some concept of "error levels" and Zod 4 will likely include some concept of a "warning".

z.string().min(5, { level: 'warn' })

That said, I don't love the idea of a warning on a type check:

z.string({ level: 'warn' })

Because this introduces the possibility to the result of .parse to disagree with the inferred static type. The point of something like Zod is to provide aa guarantee that the static & runtime values agree with each other, so this is a no-go.

I'd be willing to entertain something like this:

.safeParse(data:unknown): { success: true; data: T; } | { success: false; error: ZodError; data: unknown }

In this scenario, data is provided as unknown on the success: false case because no guarantees can be made about its type.

@m10rten
Copy link
Contributor

m10rten commented May 3, 2024

I suppose you could make a warn function that takes a zod schema and never errors, but that destroys your request, so the way I see it, this could be useful but should always be extended behaviour and not default.

Thanks for the response! Not sure I understand what you mean by destroys your request. If I understand correctly I don't believe that's what I want; I'd still want to error when "essential data" is not present. Ideally I'd want to be able to parse with a single api (i.e. multiParse, or a better name) and define whether a schema throws or warns inline if validation fails.

And to be explicit about my suggestion - it would be an extension, not default. I'd want to add .warn() and .assert() (or other names) to schemas to modify behavior, when parse and safeParse are called without these additions I'd expect the current behavior. Unfortunately I don't think it would be possibly to make .warn() useful with parse without modifying the return type, which would be a breaking change. This is why I suggested the concept of "multiparse", to preserve backwards compatibility.

Also, I want to draw a distinction between 2 related but distinct use cases I'd want to support. The labels I'd put on each are

  • Return data alongside safeParse failures
  • Override safeParse and parse dynamically

I think a new discussion/issue is warranted for the first item as it's a separate ask. It's also the api I care more about. Can continue discussion for the second item in this thread as it is appropriately named.

Warning on type check can then be avoided using an union/or.
Else to be fair you should use a transform, that way you can parse into your type.

That would be a great feature.
Combining with a passthrough and you have a very nice, but loose, object type check'ish.

@EllAchE
Copy link
Author

EllAchE commented May 3, 2024

Zod would benefit from some concept of "error levels" and Zod 4 will likely include some concept of a "warning".

z.string().min(5, { level: 'warn' })

That said, I don't love the idea of a warning on a type check:

z.string({ level: 'warn' })

Because this introduces the possibility to the result of .parse to disagree with the inferred static type. The point of something like Zod is to provide aa guarantee that the static & runtime values agree with each other, so this is a no-go.

I'd be willing to entertain something like this:

.safeParse(data:unknown): { success: true; data: T; } | { success: false; error: ZodError; data: unknown }

In this scenario, data is provided as unknown on the success: false case because no guarantees can be made about its type.

@colinhacks Thanks for the response! Happy to hear about some related features on the roadmap. Ensuring a match with valid static typing makes sense. Unknown makes sense as a default; alternatives could be not returning the mismatches at all or using any. Laying out the difference between any and unknown - you'd be forced to have type checks on the output (if using unknown) or can keep the compiler optimistically happy (if using any). Which is a better could vary by use case, though for enforcing static typing I'd say unknown is less error prone. Not returning mismatched data could be a valid use case but doesn't seem like a good default choice, it'd be similar to default with undefined.

This would achieve what I meant by "Return data alongside safeParse failures". Happy to take a look and put together a PR to add this, is there any related active development to consider/places to start?

And to address the original title of the issue for anyone coming to this thread in the future - inline overriding of throw/warn behavior is discussed above but conversation here is about returning data alongside safeParse failures.

@EllAchE
Copy link
Author

EllAchE commented May 6, 2024

@m10rten @colinhacks put together a draft PR as a starting point on a fork of the repo. https://github.com/EllAchE/zod/pull/2/files. Wanted to share for further discussion.

Implementation here ^ just types the return result of any safeParse as Output | unknown. To implement useful static typing we'd need to introduce something similar to the concept of warn/assert I discussed previously. With an assert check in place we could be assured a schema's parsed type matches the static typing (else an error is thrown), while warn would introduce unknown type to that schema (suppressing an error).

Alternatively we could get static typing by:

  • returning status codes at each sub schema
  • returning a union of possible response types mapped to possible error response types
  • possibly something better?

Some other notes:

  • This change (as is) would disable abort behavior for objects/arrays, meaning we always parse the full object. Thoughts on how essential the abort feature is vs. extended the parse capability to safeParse? Preserving abort behavior introduces additional typing/major complexity, I might suggest splitting "validation only" safeParse from "transform" safeParse if maintaining the early exit behavior is essential. There are other approaches (i.e. configuring abort under certain conditions) that could be considered too.
  • The concept of warn/assert is best paired with a default throw/warn parser. A warn is useful when the default behavior is to throw an error, while an assert is useful when the default is to return a warning.

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

3 participants