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

Support for NOT operator #41

Open
Oudwins opened this issue Oct 31, 2024 · 8 comments · May be fixed by #76
Open

Support for NOT operator #41

Oudwins opened this issue Oct 31, 2024 · 8 comments · May be fixed by #76

Comments

@Oudwins
Copy link
Owner

Oudwins commented Oct 31, 2024

These are methods that are not present in Zod or Yup but might be worth considering including in Zog.

Use cases are mainly for validating input does not have special characters, for example for usernames, or for slugs, etc

What do you all think?

@Oudwins
Copy link
Owner Author

Oudwins commented Oct 31, 2024

These would be just quality of life since you can already do this with String().Test(z.TestFunc()) or with regexes

@Jh123x
Copy link

Jh123x commented Feb 7, 2025

Can I check instead of doing this, should we also allow a .Not operator that inverts the current tests inside the schema?

@Oudwins
Copy link
Owner Author

Oudwins commented Feb 7, 2025

That would be ideal yes. However, Its not possible to do with the current architecture and Zod doesn't have it. So I am inclined to prioritize other features for the time being. But this would be cool to add.

Before anything we would have to decide on the exact behaviour of the API. I think in playwright's API (which does have this) the not only applies to the next test and not to the ones following it. That seems like the most intuitive:

// Only contains is inverted rather than both contains and min
s := z.String().Not().Contains("x").Min(3)

Not operator also has the issue of allowing users to create weird states unless we somehow change the current behaviour a lot:

s := z.String().Not().Required() // this shouldn't really be possible since we already have optional for that.
s := z.String().Min(3).Not() // this also shouldn't be possible but we won't have any way to tell the user that this is wrong
// Same with pre & post transforms:
s := z.String().Not().Trim()

The only behaviour I can think of for this is making the not apply to the next relevant test. Making these two schemas equivalent

s := z.String().Not().Trim().Min(3)
s := z.String().Trim().Not().Min(3)

@Oudwins
Copy link
Owner Author

Oudwins commented Feb 7, 2025

One thing I forgot to mention. We also would need to define how not would interact with something like super refine where you as the user manually create errors:

z.String().Not().Test(z.???(func(val any, ctx z.Ctx) {
   ctx.AddIssue(....)
})

I think most likely it doesn't do anything. I.e if you are manually creating the errors not can't really work so it should just do nothing or even the behaviour I describe earlier where it will apply to the next test

@Jh123x
Copy link

Jh123x commented Feb 8, 2025

This is what I currently have in mind.

Using your snippet as an example.

z.String().Not().Test(z.(func(val any, ctx z.Ctx) bool {
   ctx.AddIssue(....)
})

After the not operation, we can do something like this when inserting the value.
Instead of inserting it directly, we can write a wrapper that will flip the boolean.

func (s *StringSchema) Test(t p.Test) *StringSchema {
	if s.isNot {
		// Flip the validation logic
		t.ValidateFunc = func(val any, ctx ParseCtx) bool {
			return !t.ValidateFunc(val, ctx)
		}
	}

	return append(testArr, t)
}

If we flip the boolean this way, it does not matter what the user passed in as a validation function.
For the Not Operation, I'm not sure if it is better to only allow it for Tests and force the case using the syntax below

Alternatively, the syntax can also be like this

func (s *StringSchema) Not(t p.Test) *StringSchema

We allow the user to pass a test where we will flip the conditional

@Oudwins Oudwins changed the title Does NOT contain for strings Support for NOT operator Feb 8, 2025
@Oudwins
Copy link
Owner Author

Oudwins commented Feb 8, 2025

I think this is a good approach. I thought about it a bit more yesterday and came to similar conclusions to you.

I think the first API is better. The second would only work with custom tests and I think that if we are going to make the effort to implement this we would want it to work with every test.

Another idea for an API but I think its not quite as orgonomic so we should do the first:

z.String().Contains("hello", z.Not)

So assuming we pick this API

z.String().Not().Min(3)

I would like a private method inside all the schemas that adds a new test and that handles flipping the test func in the case that the schema is in "not" mode. This way all of this is handled as we build the schema and it should not impact validation performance.

This API would have the following limitations

  • It would apply to the next test as mentioned before
  • It will only flip the boolean so if you add errors manually with ctx it won't touch those

The last thing I would want to decide on is how this would interact with error messages and i18n

I think the lazy solution is to say "if you are using not you have to customize the error yourself". But I'm not a fan.

The other approach I can think of is that we add a not flag somewhere in the test (probably a param) and use that to customize the error message.

@Jh123x
Copy link

Jh123x commented Feb 8, 2025

I think this is a good approach. I thought about it a bit more yesterday and came to similar conclusions to you.

I think the first API is better. The second would only work with custom tests and I think that if we are going to make the effort to implement this we would want it to work with every test.

Another idea for an API but I think its not quite as orgonomic so we should do the first:

z.String().Min(3, z.Not)
So assuming we pick this API

z.String().Not().Min(3)
I would like a private method inside all the schemas that adds a new test and that handles flipping the test func in the case that the schema is in "not" mode. This way all of this is handled as we build the schema and it should not impact validation performance.

This API would have the following limitations

  • It would apply to the next test as mentioned before
  • It will only flip the boolean so if you add errors manually with ctx it won't touch those

The last thing I would want to decide on is how this would interact with error messages and i18n

I think the lazy solution is to say "if you are using not you have to customize the error yourself". But I'm not a fan.

The other approach I can think of is that we add a not flag somewhere in the test (probably a param) and use that to customize the error message.

For the built-in method, adding error messages for preconfigured methods might be better.
If there is a custom test, the user can input the error messages.

Let me try to make a MR and see if it is ok

@Jh123x Jh123x linked a pull request Feb 8, 2025 that will close this issue
3 tasks
@Jh123x
Copy link

Jh123x commented Feb 8, 2025

I've added an MR for the not operator, I feel that this can be extended to other schemas as well.

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 a pull request may close this issue.

2 participants