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

String .endsWith & .startsWith could perhaps be typed more strongly #1747

Open
alii opened this issue Dec 23, 2022 · 9 comments
Open

String .endsWith & .startsWith could perhaps be typed more strongly #1747

alii opened this issue Dec 23, 2022 · 9 comments
Labels
enhancement New feature or request

Comments

@alii
Copy link
Contributor

alii commented Dec 23, 2022

If the passed value was a literal, we could have a generic in ZodString that will include a prefix / suffix.

Use case:
At the moment, I am using the code below to validate that a value is a specific ID format that has a constant prefix

export function id<P extends IdPrefixes>(prefix: P) {
	return z
		.string()
		.refine((value): value is `${P}_${string}` => pika.validate(value, prefix));
}

Imagine if this could be done with just .startsWith and .endsWith — ZodString could then have a single generic of the output, which would default to just a string and can be made more specific with these two methods.

CleanShot 2022-12-23 at 14 02 47@2x

Should note that I am not sure if this is a good idea or not, but I wanted to just throw the idea out either way — it's a use case I have very frequently and always use .refine to solve.

@santosmarco-caribou
Copy link
Contributor

I've opened a PR for that. Let's see what people think.

#1751

@colinhacks
Copy link
Owner

colinhacks commented Dec 24, 2022

I have some reservations discussed here: #1730 (comment)

@JacobWeisenburger
Copy link
Contributor

I really like the idea of better static types for .startsWith and .startsWith. But I think @colinhacks is right, it would be quite hard to do this in a backwards compatible way.

@alii or @santosmarco-caribou, do you have some ideas on how to address backwards compatibility?

@santosmarco-caribou
Copy link
Contributor

One way would be to add an overload to these methods (almost like I did on my PR for ZodRequired).

We could leave the current implementation at the top of the overloads so the users will not feel any difference. This type of "exact" inference would be turned off by default.

Then, as a second overload, we add a definition that forces the user to pass true in one of the properties of the options object, turning the inference on.

Something like this:

startsWith(prefix: string, options?: { message?: string; exact?: never }): ZodString
startsWith<Start extends string>(prefix: Start, options: { message?: string; exact: true }): ZodString<Start, End>
startsWith(prefix: string, options?: { message?: string; exact?: true }): ZodString {
  // no change in implementation, the `exact` is solely for turning the inference on.
}

@JacobWeisenburger
Copy link
Contributor

I like it.

Any objections/reservations @colinhacks?

@tachibanayui
Copy link

One way would be to add an overload to these methods (almost like I did on my PR for ZodRequired).

We could leave the current implementation at the top of the overloads so the users will not feel any difference. This type of "exact" inference would be turned off by default.

Then, as a second overload, we add a definition that forces the user to pass true in one of the properties of the options object, turning the inference on.

Something like this:

startsWith(prefix: string, options?: { message?: string; exact?: never }): ZodString
startsWith<Start extends string>(prefix: Start, options: { message?: string; exact: true }): ZodString<Start, End>
startsWith(prefix: string, options?: { message?: string; exact?: true }): ZodString {
  // no change in implementation, the `exact` is solely for turning the inference on.
}

I think this is a breaking change because the second argument is message?: string | {message?: string }. It should be:

startsWith(prefix: string, options?: string | { message?: string; exact?: never }): ZodString
startsWith<Start extends string>(prefix: Start, options: { message?: string; exact: true }): ZodString<Start, End>
startsWith(prefix: string, options?: string | { message?: string; exact?: true }): ZodString {
  // no change in implementation, the `exact` is solely for turning the inference on.
}

IMO, I prefer creating a separate method for template literals because it looks cleaner when chaining methods:

const schemaUrl = z
  .string()
  .startsWith("https://", {message: "Protocol must be https!", exact: true})
  .endsWith("__schema.json", {exact: true})
  .max(255);

const schemaUrl = z
  .string()
  .startsWithLiteral("https://", "Protocol must be https!")
  .endsWithLiteral("__schema.json")
  .max(255);

@alii
Copy link
Contributor Author

alii commented Dec 25, 2022

Edit: Oops forgive me, the proposal below was already mentioned by @tachibanayui in the PR 😆

I just had a thought, perhaps the existence of z.ExactString could resolve this...

For example, ZodString could be the default for all z.string() and then when calling .endsWith & .startsWith we return a z.ExactString<Start, Mid, End> as documented in #1730

@santosmarco-caribou
Copy link
Contributor

santosmarco-caribou commented Dec 26, 2022

Yeah, I like the idea of a ZodTemplate (as in "template string") as well.

You pass an array of components (mainly ZodLiterals but we can extend it to receive other types as well) to it (or even raw values), and the inference results in a template string.

image

But that's a bit complex to handle.

But anyway, just wanted to share this idea too. Back to our ZodString, the new methods in ZodString make sense to me. I agree with it and can volunteer to fix my PR if that works for people.

I don't like the idea of a z.ExactString, though, because we would have another Zod class that does the exact same validation. That's some repetition there. Every time we add a new method to ZodString we will need to update ExactString as well.

@tachibanayui
Copy link

tachibanayui commented Dec 26, 2022

I made a type that returns ZodExactString<Start, Mid, End> or ZodString like @alii said without creating 2 classes here

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

No branches or pull requests

5 participants