sinclairzx81 / typebox

Json Schema Type Builder with Static Type Resolution for TypeScript
Other
4.77k stars 152 forks source link

Undefined is incorrectly handled in Diff #937

Closed gsimko closed 1 month ago

gsimko commented 1 month ago

The following code demonstrates the problem (using typebox 0.32.35):

import { Value } from "@sinclair/typebox/value";
Value.Diff({test:undefined},{test:undefined})

It returns this:

[
  { type: 'update', path: '/test', value: undefined },
  { type: 'insert', path: '/test', value: undefined }
]

I'd expect it to return [] as the objects are the same.

gsimko commented 1 month ago

Not sure if related, but found a similar bug around Checking with Type.Optional. Value.Check(Type.Optional(Type.Number()), undefined) returns false

sinclairzx81 commented 1 month ago

@gsimko Hi, Thanks for reporting and apologies for the delay in reply (I've been exceptionally busy of late)

I'd expect it to return [] as the objects are the same.

Yes, you're quite right. I think the diffing logic needs to factor the presence of the key, not whether the value is undefined. Ill flag this as a bug and try look at this as soon as I can (it's a good find!)

Not sure if related, but found a similar bug around Checking with Type.Optional.

This is actually ok. The Type.Optional modifier is only applicable in the context of object properties. It's actually a marker for the ? property syntax. When used outside of properties, the ? syntax yield an error.

// incorrect
const a?: number = 1 // bad syntax

// correct
const a: number | undefined = 1 // ok

const A = Type.Union([Type.Number(), Type.Undefined()])
sinclairzx81 commented 1 month ago

@gsimko Hi,

This should be fixed on revision 0.33.3. Have implemented a couple of extra optimization in this revision also. Should be a bit faster :)

Thanks again for reporting Cheers S