Skip to content

fix: avoid calling property default factory twice in Value.Default for objects#1612

Merged
sinclairzx81 merged 1 commit into
sinclairzx81:mainfrom
chatman-media:fix/default-factory-double-call
Jun 10, 2026
Merged

fix: avoid calling property default factory twice in Value.Default for objects#1612
sinclairzx81 merged 1 commit into
sinclairzx81:mainfrom
chatman-media:fix/default-factory-double-call

Conversation

@chatman-media

Copy link
Copy Markdown
Contributor

Problem

Value.Default processes object schemas by iterating each property and resolving a default when the current value is undefined. The implementation called FromType twice for each property that needed a default:

  1. Once to compute propertyValue (used only to check whether the result is an unassignable undefined)
  2. Again to actually assign value[key]
// src/value/default/from_object.ts (before fix)
const propertyValue = FromType(context, type.properties[key], value[key])  // call 1
const isUnassignableUndefined = ...
if (isUnassignableUndefined) continue
value[key] = FromType(context, type.properties[key], value[key])           // call 2 — bug

For static defaults (default: 42) this double-call was harmless. For function-based defaults (default: () => …) the factory ran twice per absent property: the first return value was silently discarded and the second was assigned.

This is a real bug for any factory that produces unique values on each call — auto-increment IDs, timestamps, UUIDs, or any stateful generator:

let counter = 0
const T = Type.Object({
  id: Type.String({ default: () => `id-${++counter}` }),
})

Value.Default(T, {})
// Expected: { id: 'id-1' }, counter === 1
// Actual:   { id: 'id-2' }, counter === 2   ← factory called twice

Fix

Reuse the propertyValue already computed on the first call instead of invoking FromType a second time:

// src/value/default/from_object.ts (after fix)
const propertyValue = FromType(context, type.properties[key], value[key])
const isUnassignableUndefined = ...
if (isUnassignableUndefined) continue
value[key] = propertyValue   // ← reuse, no second call

Tests

Two new test cases added to test/typebox/runtime/value/default/object.ts:

  • Should Default 33 — single factory property, verifies it is called exactly once and the correct value is assigned
  • Should Default 34 — two factory properties, verifies each is called exactly once

Both tests fail before the fix and pass after. All 34,879 existing tests continue to pass.

…r objects

When `Value.Default` processes an object schema, it called `FromType` twice
for each property that needed a default: once to check whether the result was
an unassignable undefined, and again to actually assign the value.

For static defaults this was harmless (same value both times), but for
function-based defaults (`default: () => …`) the factory ran twice, wasting
a call and—more importantly—discarding the first return value while using the
second. This caused incorrect results for generators that produce unique values
(auto-increment IDs, timestamps, etc.).

The fix reuses the `propertyValue` already computed on the first call instead
of invoking `FromType` a second time.
@sinclairzx81

Copy link
Copy Markdown
Owner

@chatman-media Hi, thanks for the PR!

This was a excellent find! The update is both a fix and a optimization!! :) Nice work!

Will merge through and publish on 1.2.7 shortly

@sinclairzx81 sinclairzx81 merged commit 24d2eee into sinclairzx81:main Jun 10, 2026
3 checks passed
@sinclairzx81 sinclairzx81 mentioned this pull request Jun 10, 2026
@sinclairzx81

Copy link
Copy Markdown
Owner

@chatman-media Updates have been published out to typebox@1.2.7

Thanks again :) All the best!
S

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