Skip to content

Conversation

@orimay
Copy link

@orimay orimay commented Nov 24, 2025

Continuation of the #1460

@sinclairzx81
Copy link
Owner

@orimay Hi!

Thanks for keeping this one going.


So, just been going through the tests. I note some of the logic around optional / undefined property handling can be quite complex (the Value.* API has been notoriously tricky to get right), but will want to try keep current value generation behaviors as they are while still bringing in these updates.

From a PR standpoint, will want ensure additive tests only (23, 24), but retain the existing tests as they are.

Tests


This one is good, we would expect a default value of undefined to be generated from the annotation.

Test('Should Default 23', () => {
  const X = Type.Object({ x: Type.Undefined({ default: undefined }) })
  const R = Value.Default(X, {})
  Assert.IsEqual(R, { x: undefined })
})

This one just needs an update to correctly check optional / variance handling for Undefined

Test('Should Default 24', () => {
  // const X = Type.Object({ x: Type.Optional({ default: undefined }) }) // <--- no embedded Undefined
  const X = Type.Object({ x: Type.Optional(Type.Undefined({ default: undefined })) })
  const R = Value.Default(X, {})
  Assert.IsEqual(R, {})
})

Unfortunately, will need to revert these as updates here would signal a breaking change. I think the property generation logic should be something like ...

If the default annotation is NOT present OR the property is NOT TUndefined then no key should be generated

But see how you go, mostly just want to keep these existing tests as is.

Test('Should Default 7', () => {
  const T = Type.Object({
    x: Type.Number(),
    y: Type.Number()
  })
  const R = Value.Default(T, {})
  Assert.IsEqual(R, { x: undefined, y: undefined }) // <-- unexpected key generation
})
Test('Should Default 8', () => {
  const T = Type.Object({
    x: Type.Number({ default: 1 }),
    y: Type.Number()
  })
  const R = Value.Default(T, {})
  Assert.IsEqual(R, { x: 1, y: undefined })  // <-- unexpected key generation
})
Test('Should Default 9', () => {
  const T = Type.Object({
    x: Type.Number({ default: 1 }),
    y: Type.Number()
  })
  const R = Value.Default(T, { x: 3 })
  Assert.IsEqual(R, { x: 3, y: undefined })  // <-- unexpected key generation
})

This would apply to the other updated tests too. Additive tests only.


The code updates look ok, but just need some additional guards to check specifically for properties of TUndefined. This should enable the other tests to pass.

Let me know how you go, if you have questions, feel free to ping!
Cheers
S

@orimay orimay force-pushed the main branch 3 times, most recently from a3519a3 to 92ba690 Compare November 27, 2025 16:36
@orimay
Copy link
Author

orimay commented Nov 27, 2025

Made it only populate the key what there's a default specified and the key is not optional

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