fabian-hiller / valibot

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

Invalid type error while trying to parse process.env object #587

Closed vladshcherbin closed 3 months ago

vladshcherbin commented 3 months ago

Code:

import process from 'node:process'
import { object, parse, string } from 'valibot'

console.info(typeof process.env)
console.info(process.env.NODE)
console.info('---')

parse(
  object({
    NODE: string()
  }),
  process.env
)

Result:

image
parse(
  object({
    NODE: string()
  }),
  { ...process.env }
)

valibot - 0.31.0-rc.0 node - 20.10.0

vladshcherbin commented 3 months ago

Cause: Last check from object doesn't pass.

fabian-hiller commented 3 months ago

I made the check stricter to only match plain objects. I don't know why process.env is not a plain object. Object.getPrototypeOf(process.env).constructor.name returns an empty string. Not sure if we should make the check less strict because of this.

{ ...process.env } works because it returns a plain object with all the properties of process.env.

vladshcherbin commented 3 months ago

I also have no idea and it'll be strange if it doesn't pass.

Out of interest quick test:

fabian-hiller commented 3 months ago

If others encounter the same problem, I would like to hear their opinions and whether the workaround is sufficient.

fabian-hiller commented 3 months ago

From #602. Happy to hear your feedback.

There are other problems with the current plain object check, for example in #587, but I am not sure if Valibot is the problem. Normally, this should not be a problem. We could remove the input.constructor === Object check to make it less strict, but then any object that is not null will be accepted. In practice, this is usually not a problem because we never return the original object. So, strictly speaking, the validation is still safe. Therefore, I might make the validation less strict so as not to have to deal with special JS runtime behavior that we can't control.

One problematic case that just came to my mind that could lead to unexpected behavior is our looseObject and objectWithRest schemas, as they can add unexpected data to the output if the wrong object types are passed.

fabian-hiller commented 3 months ago

This is fixed in v0.31.0-rc.6

vladshcherbin commented 3 months ago

Perfect, thank you!

I think the fix is good until a valid non-false object assumption is found 👍