fabian-hiller / valibot

The modular and type safe schema library for validating structural data 🤖
https://valibot.dev
MIT License
6k stars 186 forks source link

Add built in type guards #11

Closed JoakimDeak closed 1 year ago

JoakimDeak commented 1 year ago

Usually when using zod a common pattern I see is defining a type guard around the safeParse that would look something like:

type Data = z.infer<typeof dataSchema>
const isData = (input: unknown): input is Data => {
  return dataSchema.safeParse(input).success
}

So I would suggest adding something like an is function directly in the library to fill this function so that users don't have to create these wrapper functions.

For an example let's say you have a React component that listens to messages and if it's a specific type of message then it sets some local state. Without a built in type guard it would look something like this:

const messageDataSchema = object({
  eventId: literal('exampleEventId'),
  id: string(),
})

type MessageData = Output<typeof messageDataSchema>

const isMessageData = (input: unknown): input is MessageData => {
  return safeParse(messageDataSchema, input).success
}

const Component = () => {
  const [id, setId] = useState('')
  useEffect(() => {
    const handler = ({ data }: MessageEvent['data']) => {
      if (isMessageData(data)) {
        setId(data.id)
      }
    }
    window.addEventListener('message', handler)
    return () => {
      window.removeEventListener('message', handler)
    }
  }, [])
}

So we create the schema, infer that to a type, use the type to create a type guard, and then we can use that type guard in our message handler. Alternatively if we have built in type guards we can do:

const messageDataSchema = object({
  eventId: literal('exampleEventId'),
  id: string(),
})

const Component = () => {
  const [id, setId] = useState('')
  useEffect(() => {
    const handler = ({ data }: MessageEvent['data']) => {
      if (is(messageDataSchema, data)) {
        setId(data.id)
      }
    }
    window.addEventListener('message', handler)
    return () => {
      window.removeEventListener('message', handler)
    }
  }, [])
}

Now we create the schema and use that directly with the type guard which results in less code for the user to write. Of course we could use safeParse directly and use its return value and be type safe like this:

const messageDataSchema = object({
  eventId: literal('exampleEventId'),
  id: string(),
})

const Component = () => {
  const [id, setId] = useState('')
  useEffect(() => {
    const handler = ({ data }: MessageEvent['data']) => {
      const res = safeParse(messageDataSchema, data)
      if (res.success) {
        setId(res.data.id)
      }
    }
    window.addEventListener('message', handler)
    return () => {
      window.removeEventListener('message', handler)
    }
  }, [])
}

Personally I would prefer following the more standard format of type guards being their own functions as in the first two examples. With the third example, while working without issues, we split our checking over multiple lines and we don't narrow the type of our argument, instead we create a new variable to hold the typed data which I would also argue is not preferred.

fabian-hiller commented 1 year ago

Can you add a practical code example to the issue that explains the benefit of the type guard?

fabian-hiller commented 1 year ago

Thank you for the extended example. I now understand the benefit and am giving it some thought. The only problem I currently see is that object removes unknown keys of data, ensuring that the outgoing data of the schema matches the type definition after validation. If we only apply a type guard, data matches the type definition even though the object may contain unknown keys, which is problematic when data is written to a database, for example. When we implement this feature, it will be important to communicate this and highlight it in the documentation.

fabian-hiller commented 1 year ago

It might also help to use a name other than is for the type guard feature. For example, extends as in TypeScript could be suitable. However, this is currently just an idea. Feel free to let me know what you think about it.

JoakimDeak commented 1 year ago

object removes unknown keys of data

I think this is a great reason to offer a way to type check without modifying the data, and I absolutely agree that it should be made clear in the documentation. I think renaming it to extends is a good idea as well since that captures the idea better and is more in line with how the extends keyword works in typescript

fabian-hiller commented 1 year ago

Will think about it and then maybe implement it the next few days. Thanks for your feedback on this!

JoakimDeak commented 1 year ago

I think renaming it to extends is a good idea

I just went to make this change but it's already a reserved keyword in js so that is not actually possible

fabian-hiller commented 1 year ago

Do you have ideas for alternative names besides is?

fabian-hiller commented 1 year ago

https://valibot.dev/guides/parse-data/#type-guards