"Value.Clean" doesn't work with Unions of objects with prohibited additionalProperties about typebox HOT 3 CLOSED

@ialexi-bl Hi, thanks for reporting

I'm probably going to leave this functionality "as is" as this issue is caused by the additionalProperties: false constraint being applied to the object type (meaning the union variant fails the prerequisite check (no matching variant) for the given value). If you remove the additionalProperties: false constraint, calling Clean will remove these excess properties.

Note that the Clean and Default functions may return invalid values (and both return unknown for this reason) and there is a requirement to Check the value before using it. In terms of usage, you should implement Clean with the expectation it will fail, and handle failure cases correctly with Check.

In terms of providing a fix here, auto removing the additionalProperties: false constraint for object types per variant check would be very expensive (requiring a full clone at a minimum of the type), and not extend well to intersection and other structured types with constraints (from which there may be other caveats to consider). The current implementation tries to keep the Clean algorithm simple and fast, but does make trade offs in places, and doesn't hide some of the issues caused by schema constraints.

Workaround

The following implements logic to recursively discard the additionalProperties false constraint and implements a custom Clean function.

import { Value, IsObject, IsArray } from '@sinclair/typebox/value'
import { TSchema, Type } from '@sinclair/typebox'

// ----------------------------------------------------------------------------
// Discard (Recursively Discards Properties)
// ----------------------------------------------------------------------------
export function DiscardKey(value: unknown, key: PropertyKey): unknown {
  if(!IsObject(value) || IsArray(value)) return value
  const { [key]: _, ...rest } = value
  return rest
}
export function DiscardKeys(value: unknown, keys: PropertyKey[]) {
  return keys.reduce((acc, key) => DiscardKey(acc, key), value)
}
export function DiscardKeysRecursive<T>(value: T, keys: PropertyKey[]): T {
  const discarded = DiscardKeys(value, keys)
  return (
    IsArray(discarded) ? discarded.map(value => DiscardKeysRecursive(value, keys)) :
    IsObject(discarded) ? {
      ...Object.getOwnPropertyNames(discarded).reduce((acc, key) => ({ ...acc, [key]: DiscardKeysRecursive(discarded[key], keys) }), {}),
      ...Object.getOwnPropertySymbols(discarded).reduce((acc, key) => ({ ...acc, [key]: DiscardKeysRecursive(discarded[key], keys) }), {})
    } : discarded
  ) as never
}
// ----------------------------------------------------------------------------
// Clean (Custom Clean Pipeline)
// ----------------------------------------------------------------------------
export function Clean(schema: TSchema, value: unknown) {
  const discarded = DiscardKeysRecursive(schema, ['additionalProperties'])
  return Value.Clean(discarded, value)
}
// ----------------------------------------------------------------------------
// Usage
// ----------------------------------------------------------------------------
const T = Type.Union([
  Type.Null(),
  Type.Object({ x: Type.Number() }, { additionalProperties: false }),
])

const encoded = Clean(T, { x: 1, y: 2 })

console.log(encoded) // { x: 1 }

So the above modifies the schematics prior to calling Clean. It's not ideal, but it is inline with the semantics of how Clean works. You may be interested in the Cast function however which is loosely the inverse of Clean and will optionally remove properties if the additionalProperties constraint if applied.

import { Value } from '@sinclair/typebox/value'
import { Type } from '@sinclair/typebox'

const T = Type.Union([
  Type.Null(),
  Type.Object({ x: Type.Number() }, { additionalProperties: false }),
])

const encoded = Value.Cast(T, { x: 1, y: 2 })

console.log(encoded) // { x: 1 }

Will close off this issue for now as things are working as per current design.
Hope this helps
S

from typebox.

@sinclairzx81, thank you for a quick response. Another idea before leaving this issue: maybe instead of removing additionalProperties: false constraint recursively, adding some kind of an internal flag to Check which would ignore additionalProperties for such cases would be an option?

from typebox.

@ialexi-bl Heya

Another idea before leaving this issue: maybe instead of removing additionalProperties: false constraint recursively, adding some kind of an internal flag to Check which would ignore additionalProperties for such cases would be an option?

The problem is that Clean calls Check internally for Union determination (and Check works entirely off the schematics). So there isn't really an option to conditionally ignore the constraint (The Check function would need to be reimplemented). Also, I'm very hesitant to "mix and match" assertion logic based on additional configuration criteria (i.e. ignoreConstraint) as the criteria for assertion is configured (encoded) by way of the type itself....this is just a hard rule in TypeBox to tame complexity.

I'll give this some thought, and do agree that excess properties should be omitted irrespective of the constraint, but to achieve, it would require a overhaul of TB's internal logic for union variant matching. The Cast function currently uses a probabilistic match to do union determination (as best as it can), a similar technique may be applicable for Clean...

For now, explicitly omitting the constraint from the type is the recommendation.
Hope this brings a bit more insight
S

from typebox.