sinclairzx81 / typebox

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

Encoding does not ignore undefined values #792

Closed ayazhafiz closed 5 months ago

ayazhafiz commented 5 months ago

Hi! First off, thanks for a great library. I noticed that when encoding a type, the encoding function for a key's value in an object seems to run regardless of what the value is. For example, given

const TDate = Type.Transform(Type.String())
  .Decode((value) => new Date(value))
  .Encode((value) => value.toISOString());

const TO = Type.Object({ a: Type.Optional(TDate) });
type O = StaticDecode<typeof TO>;

const x: O = { a: undefined };

Value.Encode(TO, x);

The encoding function runs over undefined, and rightfully throws. I would expect the encoding function not to be run because the value is not the encoded TDate.

Is this the expected behavior? I realize I could also clean the object before running the encoder, but I'm not sure that's safe if the decoded and encoded values are of two different shapes.

sinclairzx81 commented 5 months ago

@ayazhafiz Hi,

Yeah, this is expected behavior (at least under the current design). Transforms work by using a 2-phase approach that will Map() and Check() in different orders depending on if you call Decode() or Encode() (and where Map() means running your Transform callback)

Decode() -> Check() -> Map()

Encode() -> Map() -> Check()

These phases are run after one another as distinct stages. TypeBox tries to avoid calls to Check() during the Map() phases and vice versa (so it's difficult to detect an invalid value during the Map() phase). Instead, it just trusts the user is submitting the correct value or it will throw either a TransformEncodeError for Map() or TransformEncodeCheckError for Check().


ExactOptionalPropertyTypes

So just on the following line...

const x: O = { a: undefined }; // this should be an error

Just keep in mind that { a: undefined } is actually an invalid value despite the type being optional. The type + optional means "the "a" key can be omitted, but if specified, it must be Date". TypeScript is a bit loose by default when it comes to this, but does provide the exactOptionalPropertyTypes configuration that will catch these kind of errors. Quick example at the link below with the option enabled.

TypeScript Link Here

import { Type, StaticDecode } from '@sinclair/typebox'

const TDate = Type.Transform(Type.String())
  .Decode((value) => new Date(value))
  .Encode((value) => value.toISOString());

const TO = Type.Object({ a: Type.Optional(TDate) })

type O = StaticDecode<typeof TO>;

const x: O = { a: undefined }; // caught with TS exactOptionalPropertyTypes: true

So, just mentioning this as it can have some consequences with respect to implementing Transforms (and looks to be the source of this issue). The best practice for writing Transforms is to try and write them defensively and to be a bit mindful about some of the nuance of end user TS configurations. For local codebases, you can switch on the exactOptionalPropertyTypes to catch this kind of error early. If you decide to do this, TypeBox provides a configuration option to get things lining up at runtime.

https://github.com/sinclairzx81/typebox?tab=readme-ov-file#policies

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

// Disallow undefined values for optional properties (default is false)
//
// const A: { x?: number } = { x: undefined } - disallowed when enabled

TypeSystemPolicy.ExactOptionalPropertyTypes = true

Hope this helps! S

sinclairzx81 commented 5 months ago

@ayazhafiz Heya,

Might close off this issue as things are generally working as expected. If you have any follow up questions though, feel free to ping on this thread.

All the best! S