sinclairzx81 / typebox

Json Schema Type Builder with Static Type Resolution for TypeScript
Other
4.79k stars 153 forks source link

Composite with Type.Unsafe specifying a kind generates an empty object #446

Closed ashervb closed 1 year ago

ashervb commented 1 year ago

This works on 0.28.7, but on 0.28.8+, given the following schema:

const Test = Type.Composite(
  [
    Type.Object({
      x: BigNumber(),
    }),
    Type.Object({
      y: BigNumber(),
    }),
  ],
  { $id: 'Test' }
)

With the Kind, e.g. const BigNumber = () => Type.Unsafe<bigint>({ [Kind]: 'BigInt', type: 'string' }) results in

      "Test": {
        "properties": {},
        "type": "object"
      }

Removing Kind, e.g. const BigNumber = () => Type.Unsafe<bigint>({ type: 'string' }) results in:

      "Test": {
        "properties": {
          "x": {
            "type": "string"
          },
          "y": {
            "type": "string"
          }
        },
        "required": [
          "x",
          "y"
        ],
        "type": "object"
      }
sinclairzx81 commented 1 year ago

@ashervb Hi, thanks for reporting!

The reason this is failing is due to TypeBox attempting to lookup a Kind as a valid TSchema. The TSchema check itself requires that unknown Kinds found in a type composition be registered with the TypeRegistry. If a Kind is set but not found within the TypeRegistry, the composition will fail. This is logic that was introduced on 0.28.0 with the "Indexed Access Types" feature.

You can solve this in a few ways:

Omit Kind

const BigNumber = () => Type.Unsafe({ }) // Defaults to [Kind]: 'Unsafe'

const Test = Type.Composite([
  Type.Object({ x: BigNumber() }),
  Type.Object({ y: BigNumber() })
], { $id: 'Test' })

Add to TypeRegistry

import { TypeRegistry } from '@sinclair/typebox'

TypeRegistry.Set('BigNumber', (schema, value) => false) // do check here

const BigNumber = () => Type.Unsafe({ [Kind]: 'BigNumber' })

const Test = Type.Composite([
  Type.Object({ x: BigNumber() }),
  Type.Object({ y: BigNumber() })
], { $id: 'Test' })

Add to TypeSystem

import { TypeSystem } from '@sinclair/typebox/system'

const BigNumber = TypeSystem.Type<string | bigint>('BigNumber', (schema, value) => {
  return false // do check here
})

const Test = Type.Composite([
  Type.Object({ x: BigNumber() }),
  Type.Object({ y: BigNumber() })
], { $id: 'Test' })

Note: The TypeRegistry and TypeSystem approaches are typically used in TypeBox's extension model (where you would want add custom types that are validated via the Value and TypeCompiler modules). The Unsafe approach (omitting Kind) is typically used for Ajv. The Kind symbol property is generally used by TypeBox to make reasoned decisions about a type (both during composition and validation), but I do think TypeBox could be doing a better job here. At this stage, I'm uncertain the best approach to take to make this experience a bit more intuitive (but do acknowledge the Kind property has become a bit overloaded in terms of it's utility within the library). For the most part, Kind should be treated as a interior property that shouldn't necessarily be set manually.

For 0.28.0, The Kind property will probably remain as it is so can advise on the 3 solutions above. Hope this helps! S

ashervb commented 1 year ago

Very interesting! Thanks so much for the very detailed explanation and all your hard work on Typebox.