Skip to content

Conversation

@orimay
Copy link

@orimay orimay commented Nov 21, 2025

Currently, if we run Value.Default on an object, that has a required property, that is undefined by default, TypeBox ignores it. This PR fixes this bug

@sinclairzx81 sinclairzx81 force-pushed the main branch 6 times, most recently from e5b6f6b to 6489b4c Compare November 22, 2025 08:51
@sinclairzx81
Copy link
Owner

@orimay Hi, thanks for the PR

I think this one is ok, but will want to include some tests as this change would implicate Default return values. The test module is at the link below, so will want to drop in a few tests here (just add them to the bottom of the test module and include link back to this PR via comment)

https://github.com/sinclairzx81/typebox/blob/main/test/typebox/runtime/value/default/object.ts

Also, looks like there's been some merge conflicts introduced since the PR commits (there's been a few rebases on the main branch related to a couple of documentation passes). Would be good to sync things up such that only modified files related to this PR show up, just helps narrow things to only files that have changed Apologies for the upstream changes.

Thanks again!

@orimay
Copy link
Author

orimay commented Nov 27, 2025

@orimay Hi, thanks for the PR

I think this one is ok, but will want to include some tests as this change would implicate Default return values. The test module is at the link below, so will want to drop in a few tests here (just add them to the bottom of the test module and include link back to this PR via comment)

https://github.com/sinclairzx81/typebox/blob/main/test/typebox/runtime/value/default/object.ts

Also, looks like there's been some merge conflicts introduced since the PR commits (there's been a few rebases on the main branch related to a couple of documentation passes). Would be good to sync things up such that only modified files related to this PR show up, just helps narrow things to only files that have changed Apologies for the upstream changes.

Thanks again!

@sinclairzx81, sorry, closed this once unintentionally, by force-pushing your upstream to my branch, but here's the fix with tests now: #1463

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants