sinclairzx81 / typebox

Json Schema Type Builder with Static Type Resolution for TypeScript
Other
5k stars 158 forks source link

Type.Mapped does not handle optional properties the same as Typescript #971

Closed ayhernandez-godaddy closed 2 months ago

ayhernandez-godaddy commented 2 months ago

I tried to search for any related issues/discussions but didn't find anything relevant.

Below is an example demonstrating a difference between a regular Typescript mapped type implementation, and the corresponding version using Typebox.

import { Static, Type } from '@sinclair/typebox';

// Vanilla Typescript example
type Base = {
  required: string;
  optional?: string;
};

type Mapped = {
  [K in keyof Base]: {
    type: 'String',
    value: Base[K]
  }
}
/*
// Resolves to:
type Mapped = {
  required: {
      type: 'String';
      value: string;
  };
  optional?: {
      type: 'String';
      value: string | undefined;
  } | undefined;
}
*/

// Typebox version of the above
const TBBase = Type.Object({
  required: Type.String(),
  optional: Type.Optional(Type.String())
});

const TBMapped = Type.Mapped(Type.KeyOf(TBBase), K => {
  return Type.Object({
    type: Type.Literal('String'),
    value: Type.Index(TBBase, K)
  });
});
/*
// Resolves to:
const TBMapped: TObject<{
    required: TObject<{
        type: TLiteral<"String">;
        value: TString;
    }>;
    optional: TObject<{
        type: TLiteral<"String">;
        value: TOptional<TString>;
    }>;
}>
*/

type TBMapped = Static<typeof TBMapped>;
/*
// Resolves to:
type TBMapped = {
    required: {
        type: "String";
        value: string;
    };
    optional: {
        value?: string | undefined;
        type: "String";
    };
}
*/

Expected behavior: The Typebox Mapped type makes the optional field optional and the optional.value field should be required, just like how the vanilla Typescript version does it.

Actual behavior: As demonstrated in the example above, the optional field ends up being required while the optional.value field is optional.

I'm opening this issue to get more information: is the discrepancy unintentional, or intentionally not supported, etc.

Thank you for making Typebox, I appreciate your time.

sinclairzx81 commented 2 months ago

@ayhernandez-godaddy Hi, thanks for reporting.

Unfortunately, this functionality is currently not supported due to some interesting distribution rules for keyof T depending on the context in which it is used. For mapped types, a raw keyof T will actually distribute key+modifier information from the source type, however it is possible to escape this by forcing evaluation of the keys prior to mapping. Consider the following.

TypeScript Link Here

type T = {
  a: string;
  b?: string;
  readonly c: string
}

// distribute: key + modifier

type A = { [K in keyof T]: 1 }      // type A = {
                                    //   a: 1;
                                    //   b?: 1 | undefined;
                                    //   readonly c: 1;
                                    // }

// store keys in S: modifier information lost

type S = keyof T

type B = { [K in S]: 1 }            // type B = {
                                    //   a: 1;
                                    //   b: 1;
                                    //   c: 1;
                                    // }

// parenthesis: modifier information lost

type C = { [K in (keyof T)]: 1 }    // type C = {
                                    //   a: 1;
                                    //   b: 1;
                                    //   c: 1;
                                    // }

So, the problem here is that TypeBox aligns to B and C. This is because Type.KeyOf(...) returns an evaluated union with no associated information regarding the modifiers for each key.

import { Type } from '@sinclair/typebox'

const T = Type.Object({
  a: Type.String(),
  b: Type.Optional(Type.String()),
  c: Type.Readonly(Type.String())
})

const S = Type.KeyOf(T)  // S: TUnion<[
                         //   TLiteral<'a'>,
                         //   TLiteral<'b'>,
                         //   TLiteral<'c'>
                         // ]>

To support modifier mapping, additional information would need to be encoded in the KeyOf result.

const S = Type.KeyOf(T)  // S: TUnion<[
                         //   TLiteral<'a'>,
                         //   TOptional<TLiteral<'b'>>, // wrapped
                         //   TReadonly<TLiteral<'c'>>  // wrapped
                         // ]>

However, this means that KeyOf would now distribute modifiers by default (where this may not be the intent). A possible solution would be the introduction of a new type to force evaluation providing an escape hatch.

const S = Type.KeyOf(T)    // S: TUnion<[
                           //   TLiteral<'a'>,
                           //   TOptional<TLiteral<'b'>>, // wrapped
                           //   TReadonly<TLiteral<'c'>>  // wrapped
                           // ]>

const E = Type.Evaluate(S) // E: TUnion<[ // new type
                           //   TLiteral<'a'>,
                           //   TLiteral<'b'>,  
                           //   TLiteral<'c'>
                           // ]>

So, the above outlines the current thinking here (which would also apply to the distribution of generic type arguments which use similar rules). This specific functionality is currently at edges of what is technically feasible to implement with functions. In TypeScript, the compiler does have the benefit of being able to derive types based on the context in which they are used, but with functions, it is only reasonable to return a single type irrespective of placement or context. This means libraries need to pick a reasonable default, and where TypeBox defaults to the B and C (Evaluated) in the examples given.

I think in future versions, the introduction of a Type.Evaluate makes sense as it would allow TypeBox to use distributable types as a default, but as of now, this functionality is currently deemed "out of scope" until such time as the mechanisms of Evaluate can be properly ironed out (it would be an exceptionally complex type)

Hope this brings some insight into the issue. Will close of this one for now as there isn't anything to action as of today, but will drop a consideration / review tag on this issue to pick up at a later time. Happy to discuss more on this thread if you have any thoughts or general questions around this.

All the best S

ayhernandez-godaddy commented 2 months ago

Thanks for the explanation 😄