feat: Strict Mode

[MentalGear]

I'd like to open a discussion for a proposal on a Strict Mode, i.e.: An error is returned if not at least one rule exists for every property in the object to check.

IMO, this would feature-complete vest in a sense that it could be reliably used to not only test "what's there" but also what isn't but should. e.g. if forms or data parameters change (but validation was neglect). Please let me know what you think.

[ealush]

Hey @MentalGear, thank you for taking interest in improving Vest.

That’s an interesting one, and the way I see it, it can be the other side of #931. There they ask for type safety - so that only fields that are declared initially will be allowed by typescript in the different fields. You want to make sure that all fields in the data object are covered.

I think that these two together can make up a really powerful “strict” mode.
I’ve attempted developing their type-only support a couple of time, but propagating the types implicitly throughout Vest was very difficult and I gave up. Bundling it as part of a “strict” mode that has runtime as well might make it more feasible.

I imagine something like:

import {strict} from 'vest';

const { test } = strict<{username: string}>((data) => {

  only(/*...*/); // would have to be a field in the generic / data object
  
  // would have to be a field in the generic / data object
  test('username', 'Username is required',() => {/*...*/})
});

The idea is that in runtime you create a narrowed down version of all the functions that take the field name (and possibly group name). It is also easier to handle in TS land since we don't need to implicitly propagate them all the way down. I do that explicitly within strict.

How does that sound?

[MentalGear]

Thanks for the interest @ealush and the detailed description. Indeed, type safety would also be great, but it's a bit too much intricate details for me, at least for the start.

What I imagined was a simple flag based approach during init (strict: true), that would just check if the input object has keys which are not in a vest rule. For example:

for ( const key of Object.keys(userInput) ) {
    if (suite.hasOwnProperty(key)===false) throw new Error(`Strict Mode Error: Input has "${key}" property that has no validation rule in vest`)
}

I think this would already bring a lot of value (as in the mentioned use-case above for outdated validation detection), and from there on, strict mode could be gradually improved.

Where do you think would that strict check best be placed?

[jonnytest1]

i think it may be nicer (just from the flow)

to structure it like this

// this way you can still export the return value without worry
// and and also narrow down the types
strict<{username: string}>(({test,only},data) => {

  only(/*...*/); // would have to be a field in the generic / data object
  
  // would have to be a field in the generic / data object
  test('username', 'Username is required',() => {/*...*/})
  
  //advanced mode :
  test("username",(expect)=>{
      expect.shouldBeGreaterThan(5) // throws error cause username is of type string but shoulsBeGreaterThan only makes sense with numbers 
                                                          //would also be a cool idea
  })
  
});

If you also want to me sure every property has a validator (and typescript to check their existence) you probably have to do something like

interface TypeDef{
   username:string,
   password:string
}


strict<TypeDef>(({test,only},data)=>{
   test({
        username:(name)=>expect(name).toHaveLength(5) // because if you do it functionally you can't  know if all usernames have been called by type
   })
})

For the strict types it may be worth it to switch to a ready made schema library like zod which already supports this and should be relatively easy to include

[MentalGear]

Thx for the example, but it seems like it would introduce a new method that is basically the same as suit. Also, this would mean that we would need to rename the calls when we want to use strict.

I'd rather prefer a simple flag:

userProfile: create((data, currentField, strict=true) => {

[jonnytest1]

renaming has some validity to it since if it really is supposed to be typed the api has to work differently so types can be passed along through the chain

i.e. the method "test" needs to be passed along from the typed suite function or it will not be able to provide type info

[ealush]

Hey, this is a pretty interesting discussion.

Thanks for the interest @ealush and the detailed description. Indeed, type safety would also be great, but it's a bit too much intricate details for me, at least for the start.

What I imagined was a simple flag based approach during init (strict: true), that would just check if the input object has keys which are not in a vest rule. For example:

for ( const key of Object.keys(userInput) ) {
    if (suite.hasOwnProperty(key)===false) throw new Error(`Strict Mode Error: Input has "${key}" property that has no validation rule in vest`)
}

I think this would already bring a lot of value (as in the mentioned use-case above for outdated validation detection), and from there on, strict mode could be gradually improved.

Where do you think would that strict check best be placed?

While I do see it bring some added value, I also see some of the possible pitfalls:

1. When the user fills up the form, you may not have all the fields initialized in your data object just yet, meaning that strict mode will pass immediately, if you have just enough tests to cover the current data object (even if there are other inputs in the form).
2. Introducing just strict:true or an internal strict() hook without considering the rest of its capabilities means that it will be locked with that functionality only, and we won't be able to add new capabilities to it until the next major version.
3. The current proposal requires narrowing the possible inputs to the suite, currently it can take anything. Strict mode means it can only take an object as the first argument.
4. We haven't discussed the rest of the arguments - can they also be only field names from the data object?

Thinking into your proposal, I think that strict mode should be bi-directional, meaning that strict will only pass if both are satisfied:

1. All keys in the data object must have at least one test assigned to them.
2. Each field in the test must have a matching property in the data object.

strict<TypeDef>(({test,only},data)=>{
   test({
        username:(name)=>expect(name).toHaveLength(5) // because if you do it functionally you can't  know if all usernames have been called by type
   })
})

I think that if we create our internal version of test, we don't need to actually modify its interface and the tests can run as they do, with some extra glue code added.

For the strict types it may be worth it to switch to a ready made schema library like zod which already supports this and should be relatively easy to include

Definitely if strict types is your top priority, then zod is a good library to go to. Their offer is quite different than Vest's so I can see why some would prefer one over the other.

I'll let you in on my thought process when adding features to Vest.

1. Is it justified? Does it server a real purpose?
   I'd say a strict mode is justified and makes sense. Forms are the parts of our apps that make use money, and they are also the more likely to have bugs because they are the hardest to test properly. Adding a strict mode can reduce the pains related to validation.

2. Should it be a breaking change, or can it be a new interface that does not change any existing behavior?
   Breaking changes require a major version bump, which I see as a strategic move for a library, and if possible, I try to opt to minor changes.

3. Does it have to be a part of the core export, or can it be an external export?
   Does it use Vest's internals to operate? Can it be an add-on on top of Vest?

4. What about discoverability and its learning curve?
   Will people find it easily in the docs? Will they have hard time using it?

---

I think that this needs a few extra spins to get right, but I'm already starting to iterate on it. I even mentioned this issue when talking about the future of Vest recently in a conference talk.

@MentalGear, I'll draft some simple addon that you'll be able to use on top of Vest. Seems pretty feasible without even changing the internals, it does not mean we won't eventually, but just to get the ergonomics right I want a few more iterations on this.