Skip to content

Conversation

@thedustinsmith
Copy link

I don't think undefinedable is working as intended based on my understanding - here's a
Playground demonstrating the issue.

This PR goes through and adds a check for undefinedable in the places that optional or nullish are modifying behavior.

Copilot AI review requested due to automatic review settings September 23, 2025 19:37
@dosubot dosubot bot added size:M This PR changes 30-99 lines, ignoring generated files. fix A smaller enhancement or bug fix labels Sep 23, 2025
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR fixes a bug where undefinedable schemas were not being treated consistently with other "optional"-ish schemas (optional and nullish) throughout the Valibot codebase. The issue was that undefinedable schemas were being incorrectly marked as required fields in object validation and JSON schema conversion.

  • Adds undefinedable type checks alongside existing optional and nullish checks
  • Updates object schema validation logic to properly handle undefinedable fields
  • Fixes JSON schema conversion to correctly mark undefinedable properties as non-required

Reviewed Changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
packages/to-json-schema/src/converters/convertSchema/convertSchema.ts Updates JSON schema conversion to exclude undefinedable fields from required properties
library/src/schemas/variant/variantAsync.ts Adds undefinedable check to discriminator validation logic
library/src/schemas/variant/variant.ts Adds undefinedable check to discriminator validation logic
library/src/schemas/strictObject/strictObjectAsync.ts Updates strict object validation to handle undefinedable fields properly
library/src/schemas/strictObject/strictObject.ts Updates strict object validation to handle undefinedable fields properly
library/src/schemas/objectWithRest/objectWithRestAsync.ts Updates object with rest validation to handle undefinedable fields properly
library/src/schemas/objectWithRest/objectWithRest.ts Updates object with rest validation to handle undefinedable fields properly
library/src/schemas/object/objectAsync.ts Updates object validation to handle undefinedable fields properly
library/src/schemas/object/object.ts Updates object validation to handle undefinedable fields properly
library/src/schemas/looseObject/looseObjectAsync.ts Updates loose object validation to handle undefinedable fields properly
library/src/schemas/looseObject/looseObject.ts Updates loose object validation to handle undefinedable fields properly

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@vercel
Copy link

vercel bot commented Sep 23, 2025

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Preview Comments Updated (UTC)
valibot Ready Ready Preview Comment Sep 23, 2025 7:50pm

@fabian-hiller
Copy link
Owner

Hey, this is not a bug. We currently offer optional, exactOptional and undefinedable for undefined values.

  • optional allows missing and undefined object entries
  • exactOptional only allows missing and but not undefined object entries
  • undefinedable only allows undefined object entries but not missing ones

undefinedable is not part of this list to disallow missing keys. But let me know if you think that I oversaw something.

@fabian-hiller fabian-hiller self-assigned this Sep 24, 2025
@fabian-hiller fabian-hiller added intended The behavior is expected and removed fix A smaller enhancement or bug fix labels Sep 24, 2025
@thedustinsmith
Copy link
Author

thedustinsmith commented Sep 24, 2025

Thanks for the quick response @fabian-hiller .

To be honest, I wasn't sure if it was the right change or not.

I will say, at the very least it seems like the documentation around undefinedable may be misleading? Specifically about "behaves exactly the same as optional at runtime" ... As demonstrated by this playground vs the original playground.


For some context, my goal was to achieve the behavior described in this line. I wanted test to be required on the output type, but undefinable during parsing:
image

@fabian-hiller
Copy link
Owner

fabian-hiller commented Sep 28, 2025

"behaves exactly the same as optional at runtime"

This is outdated and we forgot to update it! Sorry! We should get this right!

I wanted test to be required on the output type, but undefinable during parsing

You could write v.optional(v.string(), 'default string') or v.pipe(v.optional(v.string()), v.string()). The first one ensures that the output is always a string by using a default value in case of undefined. The second one will just influence the types but will always fail undefined inputs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

intended The behavior is expected size:M This PR changes 30-99 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants