valtiojs / valtio-zod

Validate valtio store values on the fly with zod
MIT License
0 stars 0 forks source link

Design considerations #1

Closed overthemike closed 1 hour ago

overthemike commented 1 month ago

Design Considerations

Validating properties on the store

Since valtio store values are typically mutated one at a time by assigning that one property with a new value, we will need to use .partial() on the zod instance internally whenever we validate. Zod's default is to error if a required property is missing on the schema it's validating. This might not be obvious to the end user and they may be confused as to why they may not be getting an error when only populating part of the schema. The alternative to this is to create a duplicate state object within valtio-zod that take a new value and populate the rest of the properties with their stored values before sending it off to valtio. The trade off here, however, is that we would essentially be doubling the size of the store, however zod will be able to validate required properties correctly.

Parsing

Zod has 2 (4 counting their async counterparts) methods to parse values: parse and safeParse. The parse method will throw an error if the validation fails while safeParse will instead return an object with a success and data property. Since the user will not be calling these methods directly when they mutate a value in valtio, we need to decide how we want to handle validation failures for them. Do we want to default to throwing an error or returning?

API Design

Right now, this is what I'm thinking (safeParse)...

import { schema } from 'valtio-zod'

// define schema
const UserSchema = z.object({
  fname: z.string(),
  lname: z.string()
})

const state = schema(UserSchema).proxy({
  fname: 'John',
  lname: 'Doe'
})

state.fname = 'Mike' // passes validation, update value
state.fname = 5 // does not pass validation, don't update

If we do it this way, it would make sense to create a special object property on either the state object itself or on each individual property that will give easy access to the error messages from Zod.

// when validation issues occur, set errors value on state object
state.fname?._errors = ["Must be a string"]

How might this all work

The schema function will return a Proxy to the valtio proxy store. This proxy will first validate any data for that is passed to that particular property. If the validation is a success, it will then take the value and mutate it to update the valtio store. If the validation fails, it won't update the state value in valtio but will instead populate an _error property on the zod version of that object (or throw maybe?)

Inline Zod calls

In case the user wants to do something different for a single use kind of thing, the status function should also accept a callback or promise where they can receive the value that is being update. This will allow them to receive the value and use zod inline within the callback. Not sure how well this will work.

const state = schema(value => {
  return z.object({
    fname: z.string(),
    lname: z.string()
  }).refine(......)
}).proxy(....)
dai-shi commented 1 month ago

This might not be obvious to the end user and they may be confused

Sounds like an interesting problem. Can you show some examples how they get confused?

overthemike commented 1 month ago

This might not be obvious to the end user and they may be confused

Sounds like an interesting problem. Can you show some examples how they get confused?

// all of these properties are required by default
const UserSchema = z.object({
  fname: z.string(),
  lname: z.string(),
  email: z.string().email()
})

const jsonobj = {
  fname: 'John',
  lname: 'Doe',
  email: 'email@example.com',
}

// This works because all values are both present and valid. 
// WIth Zod by itself, when you validate a schema, you usually validate
// an entire object instead of one value at a time.
UserSchema.parse(jsonobj)

UserSchema.parse({fname: 'Jane'}) // this will error because it is missing required properties

//-----------------------------//
let state = proxy({
  fname: 'John',
  lname: 'Doe',
  email: 'email@example.com',
})

// With Valtio, however, replacing the object is a no no
// as it will replace the proxy object with the new one
state = {
  fname: 'Jane',
  lname: 'Doe',
  email: 'email@example.com'
} // BAD BAD BAD

// Essentially, the two libraries conventions sort of contradict each other
state.fname = 'Jane'
state.lname = 'Doe'
state.email = 'email@example.com'

Does that make sense?

This isn't an insurmountable issue I don't think. We just need to decide if using .partial() is a worthwhile concession or if we should rebuild the object whenever a new property value is passed in and replace only the value for that property and have zod validate it.

dai-shi commented 1 month ago

// Essentially, the two libraries conventions sort of contradict each other

In this case, it doesn't conflict, if you UserSchema.parse against the merged object. But, I get the issue of timing. If a schema is like a union type, it doesn't work.

overthemike commented 1 month ago

I'm not sure what you mean . Are you talking about the merged state object within valtio? If so, at least with my current idea for an implementation, that won't actually help. The schema function will proxy to valtio and do the validations before we send values to be stored in the state object. Unless you are talking about creating a storage object within this library? Like the duplicate I talked about earlier?

dai-shi commented 1 month ago

I mean like this:

state.fname = 'Jane'
// Validate against { fname: 'Jane', lname: 'Doe', email: 'email@example.com' },
// not { fname: 'Jane' }

I'm not following every details, so there can be something I missed.

overthemike commented 1 month ago

I probably just didn't explain it very well. I was just trying to represent how zod would see things when we mutate a value. The issue is that this wrapper library will have to intercept all of the property changes before it ever goes into the state. We would either need to store a new state in this library to fetch the other property values from, or go get it from valtio, we won't know what those values are. The alternative, however, is to just use .partial() on whatever schema is passed in so that all properties become optional so we won't need to do any work that isn't necessary.

dai-shi commented 1 month ago

We would either need to store a new state in this library to fetch the other property values from, or go get it from valtio, we won't know what those values are.

My proposal was to "emulate" the new state before going to valtio, but maybe it's too difficult and too tricky.

.partial() sounds like an reasonable way to start with.

overthemike commented 1 month ago

The alternative to this is to create a duplicate state object within valtio-zod that take a new value and populate the rest of the properties with their stored values before sending it off to valtio. The trade off here, however, is that we would essentially be doubling the size of the store, however zod will be able to validate required properties correctly.

Yeah that's pretty much what I meant here. I think either way will be fine to be honest (emulation or partial). I think it'll end up being more of an implementation detail. Should be invisible to the end user either way.

How would propose we handle validation errors? Should we throw an Error? One idea is to kind of copy what Zod does and add a property (._error maybe? or _status?) in this library's proxy get trap that will store the error metadata on each property. We can store all of the error metadata in a WeakMap.

const UserSchema = z.object({
  fname: z.string(),
  lname: z.string(),
  email: z.string().email()
})

// Create the state and attach the schema
const state = schema(UserSchema).proxy({
  fname: 'John',
  lname: 'Doe',
  email: 'email@example.com'
})

console.log(state.fname) // Outputs 'John'

// assign an invalid value
state.fname = 5

console.log(state.fname) // Still outputs 'John'

// but now the property will have additional data on the _error property.
console.log(state.fname._error) 
/** Outputs
{
  error: {
    property: 'fname',
    messages: [ 'Expected string, received number'],
    lastInvalidValuePassed: 5,
    currentValue: 'John'
}
*/

The one thing I'm still trying to figure out, however, is if we don't throw an error and force the user to use a try/catch, what would be the best way have the end user to handle the errors?

dai-shi commented 1 month ago

Yeah that's pretty much what I meant here.

😅

How would propose we handle validation errors?

With valtio design decisions, we strictly avoid adding properties that can conflict with user properties, so ._error isn't acceptable. We use symbols for such cases const ERROR = Symbol(). Personally, I'd prefer throwing an error.

That said, this library doesn't need to follow 100% of valtio design decisions. If most zod users expect ._error, it might be a good option.

overthemike commented 1 hour ago

I've pushed up an initial version, but I'm a bit tired so I haven't finished everything yet.