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

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

Closed
vladshcherbin opened this issue May 22, 2024 · 7 comments
Closed

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

vladshcherbin opened this issue May 22, 2024 · 7 comments
Assignees
Labels
external Problem has external origin question Further information is requested

Comments

@vladshcherbin
Copy link

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
  • same error with looseObject and strictObject
  • no error in 0.30
  • no error with spread object copy:
parse(
  object({
    NODE: string()
  }),
  { ...process.env }
)

valibot - 0.31.0-rc.0
node - 20.10.0

@vladshcherbin
Copy link
Author

Cause: Last check from object doesn't pass.

@fabian-hiller
Copy link
Owner

fabian-hiller commented May 22, 2024

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.

@fabian-hiller fabian-hiller self-assigned this May 22, 2024
@fabian-hiller fabian-hiller added the question Further information is requested label May 22, 2024
@vladshcherbin
Copy link
Author

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

Out of interest quick test:

@fabian-hiller
Copy link
Owner

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

@fabian-hiller
Copy link
Owner

fabian-hiller commented May 27, 2024

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 fabian-hiller added the external Problem has external origin label May 27, 2024
@fabian-hiller
Copy link
Owner

This is fixed in v0.31.0-rc.6

@vladshcherbin
Copy link
Author

Perfect, thank you!

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
external Problem has external origin question Further information is requested
Projects
None yet
Development

No branches or pull requests

2 participants