sinclairzx81 / typebox

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

Partials not working as expected #369

Closed ridulfo closed 1 year ago

ridulfo commented 1 year ago

https://github.com/sinclairzx81/typebox/blob/749a6d04af2c57f86ae26428e8786882fee377d9/test/runtime/compiler/partial.ts#L6-L21

Might have found a bug in Partial which the test above does not test for. In our project we use Fastify + typebox for body validation. One of the bodies looks something like this:

const A = Type.Partial(
  Type.Object({
    X: Type.Object({
      XX: Type.Number({ default: 4 }),
    }),
    Y: Type.Object({
      YY: Type.Boolean({ default: true }),
    }),
  })
)

This object is let through.

{X: {YY: true}}

The generated typescript type is correct. But the validation is not working for us. Could you please add a test for this. Thanks! 🙏

PS: I wasn't able to set up the repo to create my own test. Hammer is broken on mac. 😞

sinclairzx81 commented 1 year ago

@ridulfo Hi!

This is unusual. I can't seem to replicate the issue locally, but you should be able to run the following your side (sorry about Hammer not working on Mac, would be interested to see what error is reported)

$ npm install ajv @sinclair/typebox
import { TypeCompiler } from '@sinclair/typebox/compiler'
import { Value } from '@sinclair/typebox/value'
import { Type } from '@sinclair/typebox'
import Ajv from 'ajv'

const A = Type.Partial(
  Type.Object({
    X: Type.Object({
      XX: Type.Number({ default: 4 }),
    }),
    Y: Type.Object({
      YY: Type.Boolean({ default: true }),
    }),
  })
)
const V = { X: {YY: true} }
console.log((new Ajv()).validate(A, V))         // false       
console.log(Value.Check(A, V))                  // false
console.log(TypeCompiler.Compile(A).Check(V))   // false

I can include a test for this, but I'm thinking this issue could be related to the Ajv configuration for useDefaults. Are you able to check and see if you have this enabled? I think in the omission of data, Ajv will populate it with useDefault, also, the YY set on X is probably interpreted as an additional property.

You can constrain your schema this way.

const A = Type.Partial(
  Type.Object({
    X: Type.Object({
      XX: Type.Number({ default: 4 }),
    }, { additionalProperties: false }), // YY was being allowed here
    Y: Type.Object({
      YY: Type.Boolean({ default: true }),
    }, { additionalProperties: false }),
  }, { additionalProperties: false })
)

Let me know if this helps S

ridulfo commented 1 year ago

You are completely right! Typebox validates correctly both in this simple example in the issue and bigger one in our project. The issue might lie in how fastify is using typebox. I'll continue looking :)

Thanks a lot @sinclairzx81 ! ⭐