sinclairzx81 / typebox

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

How to express function optional parameter? #839

Closed jrencz closed 3 months ago

jrencz commented 3 months ago

I'm trying to express a type for object that has method clone, which in my case has optional parameter.

My case boiled down to essentials is

type Serialized = {x: string, y: string};
class T {
  constructor(public x: string, public y: string) {}

  static fromSerialized({x, y}: Serialized): T {
    return new T(x, y);
  }

  // For demonstrative purposes
  cloneNotOptional(mixin: Partial<T | Serialized>): T {
    return T.fromSerialized({
      ...this,
      ...mixin,
    });
  }

  clone(mixin?: Partial<T | Serialized>): T {
    return T.fromSerialized({
      ...this,
      ...mixin,
    });
  }
}

const test = T.fromSerialized({x: 'a', y: 'b'});

// @ts-expect-error
test.cloneNotOptional();
test.clone();

open in TS playground

I'm trying to write type for clone with typebox.

I thought that something like (constructor is omitted. Partially for brevity, partially, because I don't know how to express it, and that's not the point of this issue):

const T = Type.Recursive((This) => Type.Object({
  x: Type.String(),
  y: Type.String(),
  cloneUnionWithUndefined: Type.Function([Type.Union([
    Type.Partial(This),
    Type.Undefined()
  ])], This),
  cloneUnionOfFunctionTypesWithUndefined: Type.Union([
    Type.Function([Type.Partial(This)], This),
    Type.Function([], This),
  ]),
  cloneOptional: Type.Function([Type.Optional(Type.Partial(This))], This),
}));
export type T = Static<typeof T>;

const t: T = {
  x: 'a',
  y: 'b',
  cloneUnionWithUndefined(mixin) {
    return {...this, ...mixin};  
  },
  cloneUnionOfFunctionTypesWithUndefined(mixin) {
    return {...this, ...mixin};  
  },
  cloneOptional(mixin) {
    return {...this, ...mixin};
  }
};

// @ts-expect-error
t.cloneUnionWithUndefined()

// @ts-expect-error
t.cloneUnionOfFunctionTypesWithUndefined()

// @ts-expect-error
t.cloneOptional()

All 3 of my attempts don't bring the expected result. I came to a conclusion that it may just not be supported at this point. But it's also likely: it is, I just don't know the tool good enough to figure it out. But back to my example:

I'm aware that "undefined given at position 0" vs "argument at position 0 not given" is not exactly the same so cloneUnionWithUndefined is a hack.

What cloneUnionOfFunctionTypesWithUndefined is seems the most rational to me, at least at this stage of my experience with Typebox, but it still feels kind of like a hack, because I'm not after having "union of 2 functions", but unless there's a proper way to have to express optional parameter yet (which I guess it may be), it seems like that might work like a function overload (I don't see them either)

The cloneOptional approach Is just guesswork - that's the API it might have, but I'm aware Type.Optional is about expressing keys of object, not presence or absence of something, so it should rather be a Type.OptionalParameter

sinclairzx81 commented 3 months ago

@jrencz Hi,

This is a good catch (not sure why this hadn't been implemented). I've just pushed an update to support Optional and Readonly arguments on Function and Constructor types. The following implementation is now supported on 0.32.22.

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

type T = Static<typeof T>
const T = Type.Recursive((This) => Type.Object({
  x: Type.String(),
  y: Type.String(),
  clone: Type.Function([Type.Optional(Type.Partial(This))], This),
}))

const t: T = {
  x: 'a',
  y: 'b',
  clone(mixin) {
    return {...this, ...mixin};
  }
};

t.clone()                           // ok
t.clone({ x: 'c' })                 // ok
t.clone().clone().clone({ y: 'd' }) // ok

Just as an aside. Just be a bit mindful about the use of Partial(This). Utility types (like Partial) can't operate directly on reference schematics (where This is expressed as { $ref: '...' }) and where TypeBox needs the full schematics in order to map the type as Partial.

In most cases, the Type.Deref() type can be used to de-reference schematics prior to applying utility mappings, but in the case of recursive schematics, the target schema cannot be de-referenced due it to being self referential. Just console.log(T) the schematics to check that they are satisfactory for your use case.

Hope this helps S

jrencz commented 3 months ago

It does - thank you for fixing that so quickly.

The case as I described it now works.

It turns out that what I actually try to do is a bit more complex. I guess that it's unrelated (i.e. on a plane of Typebox - it's definitely related in my case)

Here's how I modified your example so it's more like mine

type T = Static<typeof T>
const T = Type.Recursive((This) => {
  type T = Static<typeof T>;
  const T = Type.Object({
    x: Type.String(),
    y: Type.String(),
    clone: Type.Function([Type.Optional(Type.Partial(This))], This),
  });

  // 1: this is `never` - why?
  type checkInside = ReturnType<T['clone']>;

  return T;

  // 3: and if I uncomment the line below and comment out the `return T` above…
  // return Type.Unsafe<T>(T);
});

// 2: in here it's T. As expected
type checkOutside = ReturnType<T['clone']>;
// 4: …then it's `never` here as well

First thing that's likely expected, but I'm unable to explain myself thy is that is the (1): why is it a bad idea to try to infer type inside the callback of Type.Recursive? Now when I wrote it down it indeed seems like there may be something terribly wrong with what I try to do, but anyway - should it be never at this point, or is it a bug?

A word of explanation why I'd want to have it: My object not only uses this recursive clone, but also has some Opaque/Branded strings as values.

I'm using https://www.npmjs.com/package/ts-opaque

I'm aware that in Typebox there's no notion of Opaque/Branded types, so I thought I can mess a bit with the result type inside callback of Type.Recursive and make it return something that will infer a correct type, with Opaque/Branded types where needed.

Back to the flow in example: the (2) is, as expected - T
But this one also becomes never when I do the (3). Seems like a consequence of the fact that the (Broken? Invalid? Incorrectly used?) T inside callback was forced into type argument of Type.Unsafe - at least that's how I reason that to myself.

Should that extended example work or not?

jrencz commented 3 months ago

OK, my approach was clearly incorrect, but writing it down helped me figure out something that does actually work: 2-level recursive

On innermost level This is used to declare both the argument and return type of clone. On intermediate level I'm not even using This of that level - no need to. It's only so that I can apply my brands

import Opaque from 'ts-opaque';

export interface X {
  id: Opaque<string, X>;
}
export interface Y {
  id: Opaque<string, Y>;
}

type T = Static<typeof T>
const T = Type.Recursive(() => {
  type I = Static<typeof I>;
  const I = Type.Recursive((This) => Type.Object({
    x: Type.String(),
    y: Type.String(),
    clone: Type.Function([Type.Optional(Type.Partial(This))], This),
  }));

  return Type.Unsafe<I & {
    x: X["id"];
    y: Y["id"];
  }>(I);
});

type cloneReturnType = ReturnType<T['clone']>;
type x = T['x'];
type y = T['y'];

Unless this has some underwater stones, I think I figured out a way to use brands somehow

sinclairzx81 commented 3 months ago

@jrencz Heya

1: this is never - why?

So the reason you get never interior to the recursive type is due to the way TypeBox evaluates the This parameter. Recursive types work by determining the type from the callbacks return value. Recursive type are evaluated by way of higher kinded type (HKT), and where the evaluation itself is lazy. Because of this, anything occurring "before" the evaluation of the HKT is observed as never.

Unless this has some underwater stones, I think I figured out a way to use brands somehow

You look like you're on the right track. While TypeBox doesn't provide immediate support for branded types (as there is no associated runtime schematics representing the brand), you can still implement them by way of Unsafe. The following pattern is sometimes used in tandem RegExp or format strings where the String type is wrapped in an Unsafe. The brand itself is only meaningful to the TS type system.

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

const A = Type.Unsafe<string & { __brand: 'A' }>(Type.String())
const B = Type.Unsafe<string & { __brand: 'B' }>(Type.String())

type A = Static<typeof A>
type B = Static<typeof B>

function test(value: A) {}

test('A' as B) // error

It is also common to wrap types like this up in functions for re-use.


Will go ahead and close up this issue for now as the Optional (and Readonly) case is solved for on 0.32.22. Happy to continue to discuss some of the finer aspects of TypeBox if you have follow on questions, but it would be better to move general discussions and queries over to https://github.com/sinclairzx81/typebox/discussions.

All the best! S

jrencz commented 3 months ago

Thanks for the explanations and for the quick adjustment once again