sinclairzx81 / typebox

Json Schema Type Builder with Static Type Resolution for TypeScript
Other
4.65k stars 150 forks source link

transform object property encoding #859

Closed bhamon closed 2 months ago

bhamon commented 2 months ago

When using a Transform as a type for an Object property the decoding works fine. The encode and decode functions gets the proper type inference. But the encode function return value is rejected by the Check performed after transformation.

Here is a simple reproduction code:

import {Type, type StaticDecode, type StaticEncode} from '@sinclair/typebox';
import {TransformEncodeCheckError, Value} from '@sinclair/typebox/value';

const SchemaDate = Type.Number();

/** Simple number to Date transform */
const TransfromDate = Type.Transform(SchemaDate)
  .Decode(v => new Date(v))
  .Encode(v => v.getTime());

/** inferred as number */
type TransformDateEncode = StaticEncode<typeof TransfromDate>;
/** inferred as Date */
type TransformDateDecode = StaticDecode<typeof TransfromDate>;

class User {
  name: string;
  createdAt: Date;

  constructor(_name: string, _createdAt: Date) {
    this.name = _name;
    this.createdAt = _createdAt;
  }
}

/** here I use the date transform directly */
const SchemaUser = Type.Object({
  name: Type.String(),
  created_at: TransfromDate
});

const TransformUser = Type.Transform(SchemaUser)
  .Decode(v => {
    /**
     * v is inferred as {name: string, created_at: Date}
     * created_at is properly transformed *before* being passed to this function
     */
    return new User(v.name, v.created_at);
  })
  .Encode(v => {
    /**
     * v inferred as User
     * the expected return value for this function is {name: string, created_at: Date}
     * the created_at property should be transformed after being returned from this function
     */
    return {
      name: v.name,
      created_at: v.createdAt
    };
  });

/** inferred as {name: string, created_at: number} */
type TransformUserEncode = StaticEncode<typeof TransformUser>;
/** inferred as User */
type TransformUserDecode = StaticDecode<typeof TransformUser>;

{
  const data = {
    name: 'test',
    created_at: Date.now()
  };

  /** works fine */
  const user = Value.Decode(TransformUser, data);
  console.log(user);
}

{
  const user = new User('test', new Date());
  try {
    const data = Value.Encode(TransformUser, user);
    console.log(data);
  } catch (e) {
    if (e instanceof TransformEncodeCheckError) {
      console.log(e.schema);
      console.log(e.value);
      console.log(e.error);

      /*
        {
          type: "object",
          properties: {
            name: {
              type: "string",
              [Symbol(TypeBox.Kind)]: "String",
            },
            created_at: {
              type: "number",
              [Symbol(TypeBox.Kind)]: "Number",
              [Symbol(TypeBox.Transform)]: [Object ...],
            },
          },
          required: [ "name", "created_at" ],
          [Symbol(TypeBox.Kind)]: "Object",
          [Symbol(TypeBox.Transform)]: {
            Decode: [Function],
            Encode: [Function],
          },
        }

        {
          name: "test",
          created_at: 2024-05-03T06:48:27.149Z,
        }

        {
          type: 41,
          schema: {
            type: "number",
            [Symbol(TypeBox.Kind)]: "Number",
            [Symbol(TypeBox.Transform)]: {
              Decode: [Function],
              Encode: [Function],
            },
          },
          path: "/created_at",
          value: 2024-05-03T06:48:27.149Z,
          message: "Expected number",
        }
      */
    }

    throw e;
  }
}

Judging by the value returned in the TransformEncodeCheckError, it seems that the returned value of the encode function of TransformUser is indeed wrong because the created_at field as been kept as it was: a Date. Shouldn't it have been encoded by it's transform beforehand?

If I change the encode function return value to:

    return {
      name: v.name,
      created_at: v.createdAt.getDate()
    };

It works fine at run time, but the returned type is rejected by typescript:

Argument of type '(v: User) => { name: string; created_at: number; }' is not assignable to parameter of type 'TransformFunction<User, { name: string; created_at: Date; }>'.
  Call signature return types '{ name: string; created_at: number; }' and '{ name: string; created_at: Date; }' are incompatible.
    The types of 'created_at' are incompatible between these types.
      Type 'number' is not assignable to type 'Date'.ts(2345)
sinclairzx81 commented 2 months ago

@bhamon Hi, thanks for reporting!

Yeah, this is definitely a bug. I had carried out some optimization work last weekend to improve Encode/Decode object initialization performance, but it appears I'd introduced a bug which wasn't being caught by the test suite.

Have pushed a fix for this on 0.32.28. If you install this version, your repro should work as expected.

Let me know how you go. Cheers S

bhamon commented 2 months ago

Thank you for your time. It works with the new 0.32.28 version.

I also have a question regarding union encoding: does the encoding strategy always use the first element of the union? Or is there a way to enforce it (with an option maybe)?

For example I've defined a date transform as follow:

import {type StringOptions, type NumberOptions, Type, type StaticEncode} from '@sinclair/typebox';

/** Date encoded as an ISO8601-compliant string */
export const DateTimeStringTransform = (_options: StringOptions = {}) =>
  Type.Transform(
    Type.String({
      ..._options,
      format: 'date-time'
    })
  )
    .Decode(v => new Date(v))
    .Encode(v => v.toISOString());

/** Date encoded as a millisecond timestamp number */
export const DateTimeNumberTransform = (_options: NumberOptions = {}) =>
  Type.Transform(Type.Integer(_options))
    .Decode(v => new Date(v))
    .Encode(v => v.getTime());

/** Compound date codec that accepts both string and number representation */
export const DateTimeTransform = (
  _options: {string?: StringOptions; number?: NumberOptions} = {string: {}, number: {}}
) => Type.Union([DateTimeStringTransform(_options.string), DateTimeNumberTransform(_options.number)]);

/* inferred as string|number */
type DateTimeTransformEncode = StaticEncode<ReturnType<typeof DateTimeTransform>>;

export default DateTimeTransform;

The inferred encode type is string | number. It could be syntactically correct. But at run time it always returns a string because a choice has to be made regarding the proper encode to apply.

sinclairzx81 commented 2 months ago

I also have a question regarding union encoding: does the encoding strategy always use the first element of the union? Or is there a way to enforce it (with an option maybe)?

TypeBox will always return the first matching Union variant, but it's the role of the Encode function to return one of those variants. In the case of Date, to be able to conditionally Encode as either number or string, you will need the Decode function to apply a discriminator to the Date object such that Encode function can make an informed decision on how to encode later.

// Bad Decode (lost information about source type)
string  -> Date
number  -> Date

// Good Decode (union information retained for later Encode)
string -> Date & { type: 'string' }
number -> Date & { type: 'number' }

The following implements a DiscriminatedDate type that achieves the above.

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

// ---------------------------------------------------------
// DiscriminatedDate
// ---------------------------------------------------------
export type DiscriminatedDate = Date & { kind: 'string' | 'number' | ({} & string) }

const DecodeDiscriminatedDate = (value: string | number) => (
  Object.defineProperty(new Date(value), 'kind', { value: typeof value }) 
) as DiscriminatedDate

const EncodeDiscriminatedDate = (value: DiscriminatedDate) => (
  value.kind === 'string' ? value.toISOString() :
  value.kind === 'number' ? value.getTime() :
  value.getTime()
)
const DiscriminatedDate = Type.Transform(Type.Union([Type.Number(),Type.String()]))
  .Decode(DecodeDiscriminatedDate)
  .Encode(EncodeDiscriminatedDate)

// ---------------------------------------------------------
// String
// ---------------------------------------------------------
{
  const D = Value.Decode(DiscriminatedDate, '1970-01-01T00:00:12.345Z')
  const E = Value.Encode(DiscriminatedDate, D)
  console.log('string:')
  console.log('  decode:', D)
  console.log('  encode:', E)
}
// ---------------------------------------------------------
// Number
// ---------------------------------------------------------
{
  const D = Value.Decode(DiscriminatedDate, 12345)
  const E = Value.Encode(DiscriminatedDate, D) 
  console.log('number:')
  console.log('  decode:', D)
  console.log('  encode:', E)
}
// ---------------------------------------------------------
// Embedded
// ---------------------------------------------------------
{
  const User = Type.Transform(Type.Object({ created_at: DiscriminatedDate }))
    .Decode(value => ({ createdAt: value.created_at }))
    .Encode(value => ({ created_at: value.createdAt }))

  const D = Value.Decode(User, { created_at: '1970-01-01T00:00:12.345Z' })
  const E = Value.Encode(User, D)
  console.log('embedded:')
  console.log('  decode:', D)
  console.log('  encode:', E)
}
string:
  decode: 1970-01-01T00:00:12.345Z
  encode: 1970-01-01T00:00:12.345Z
number:
  decode: 1970-01-01T00:00:12.345Z
  encode: 12345
embedded:
  decode: { createdAt: 1970-01-01T00:00:12.345Z }
  encode: { created_at: '1970-01-01T00:00:12.345Z' }

Union transforms are somewhat interesting, but as a general rule, if you are transforming a Union with 10 variants, the Decode function should return a mapping all 10 in such a way the Encode function can later perform the reverse mapping.

Hope this helps, will close off this issue for now Cheers S

bhamon commented 2 months ago

Thank you for your answer.

There was indeed a lack of discriminator in my code. I've changed it to include a discriminator and a default string encoding for non-DiscriminatedDate.