rest.ts icon indicating copy to clipboard operation
rest.ts copied to clipboard

Example definition in README fails to compile in Typescript strict mode

Open jacobdr opened this issue 5 years ago • 6 comments

Copied the examples directly, I get the following error:

node_modules/rest-ts-express/dist/index.d.ts:14:11 - error TS2430: Interface 'TypedRequest<T>' incorrectly extends interface 'Request<ParamsDictionary, any, any, ParsedQs>'.
  Types of property 'query' are incompatible.
    Type 'ExtractRuntimeType<T["query"]>' is not assignable to type 'ParsedQs'.
      Type 'void | (T["query"] extends (infer T)[] ? T extends [infer A] ? [ExtractBaseType<A>] : T extends [infer A, infer B] ? [ExtractBaseType<A>, ExtractBaseType<B>] : T extends [...] ? [...] : T extends [...] ? [...] : T extends [...] ? [...] : T extends [...] ? [...] : T extends [...] ? [...] : T extends [...] ? [...] : E...' is not assignable to type 'ParsedQs'.
        Type 'void' is not assignable to type 'ParsedQs'.
          Type 'void | { [x: string]: any; }' is not assignable to type 'ParsedQs'.
            Type 'void' is not assignable to type 'ParsedQs'.

14 interface TypedRequest<T extends EndpointDefinition> extends express.Request {
             ~~~~~~~~~~~~


Found 1 error.

Stepping down some levels, the definition of:

export declare type ExtractRuntimeType<T> = T extends undefined ? void : T extends Array<infer T> ? T extends [infer A] ? [ExtractBaseType<A>] : T extends [infer A, infer B] ? [ExtractBaseType<A>, ExtractBaseType<B>] : T extends [infer A, infer B, infer C] ? [ExtractBaseType<A>, ExtractBaseType<B>, ExtractBaseType<C>] : T extends [infer A, infer B, infer C, infer D] ? [ExtractBaseType<A>, ExtractBaseType<B>, ExtractBaseType<C>, ExtractBaseType<D>] : T extends [infer A, infer B, infer C, infer D, infer E] ? [ExtractBaseType<A>, ExtractBaseType<B>, ExtractBaseType<C>, ExtractBaseType<D>, ExtractBaseType<E>] : T extends [infer A, infer B, infer C, infer D, infer E, infer F] ? [ExtractBaseType<A>, ExtractBaseType<B>, ExtractBaseType<C>, ExtractBaseType<D>, ExtractBaseType<E>, ExtractBaseType<F>] : T extends [infer A, infer B, infer C, infer D, infer E, infer F, infer G] ? [ExtractBaseType<A>, ExtractBaseType<B>, ExtractBaseType<C>, ExtractBaseType<D>, ExtractBaseType<E>, ExtractBaseType<F>, ExtractBaseType<G>] : T extends [infer A, infer B, infer C, infer D, infer E, infer F, infer G, infer H] ? [ExtractBaseType<A>, ExtractBaseType<B>, ExtractBaseType<C>, ExtractBaseType<D>, ExtractBaseType<E>, ExtractBaseType<F>, ExtractBaseType<G>, ExtractBaseType<H>] : Array<ExtractBaseType<T>> : ExtractBaseType<T>;
declare type ExtractBaseType<T> = T extends rt.Runtype ? rt.Static<T> : T extends {
    new (...args: any[]): infer T;
} ? T : T extends any[] | Function ? T : T extends {
    [k: string]: any;
} ? {
    [K in keyof T]: ExtractRuntimeType<T[K]>;
} : T;

Changing the following:

export declare type ExtractRuntimeType<T> = T extends undefined ? void :
export declare type ExtractRuntimeType<T> = T extends undefined ? never :

fixes the issue for me.

Given I am new to the library and unable to assess risk and potential breaking effect of the change, I feel a bit uncomfortable making a PR at this time.

jacobdr avatar May 09 '20 18:05 jacobdr

Another fix I tried but seems the compiler is not smart enough to know about the elimination of the potential void type:

interface TypedRequest<T extends EndpointDefinition> extends express.Request {
    body: ExtractRuntimeType<T['body']>;
    params: Tuple2Dict<T['params']>;
    query: ExtractRuntimeType<T['query']> extends void ? {} : ExtractRuntimeType<T['query']>;
}

jacobdr avatar May 09 '20 19:05 jacobdr

This works:

interface TypedRequest<T extends EndpointDefinition> extends express.Request {
    body: ExtractRuntimeType<T['body']>;
    params: Tuple2Dict<T['params']>;
    query: T['query'] extends void ? {} : NonNullable<T['query']>;
}

jacobdr avatar May 10 '20 00:05 jacobdr

I was able to reproduce the issue in the test suite, but not minimally so (yet). I’ll try and file a PR at some point in the near future

jacobdr avatar May 10 '20 21:05 jacobdr

Thanks @jacobdr . I appreciate you taking the time to investigate this issue.

hmil avatar May 13 '20 16:05 hmil

@hmil Could use your help here....

Essentially, it seems like the underlying types in the upstream @types/express repo have changed, and that is what was needed to get a minimum reproduction

You can find that work here: https://github.com/hmil/rest.ts/pull/20/commits/35ed981431c8a6204ccb0b49492bd41adacbefc4

After that, I tried to implement a fix. That work was here: https://github.com/hmil/rest.ts/pull/20/commits/bcdba81b5aecc6067467f2da777cccb964381713

However, when I did that we end up with a final compilation error:

test/fixtures/testAPIServer.ts:18:25 - error TS2345: Argument of type '(req: TypedRequest<Readonly<EmptyInitialEndpointDefinition<"PUT">>>, res: Response<any>) => Promise<void>' is not assignable to parameter of type 'RouteHandler<Readonly<EmptyInitialEndpointDefinition<"PUT">>>'.
  Call signature return types 'Promise<void>' and 'PromiseOrValue<undefined>' are incompatible.
    The types of 'then' are incompatible between these types.
      Type '<TResult1 = void, TResult2 = never>(onfulfilled?: ((value: void) => TResult1 | PromiseLike<TResult1>) | null | undefined, onrejected?: ((reason: any) => TResult2 | PromiseLike<TResult2>) | null | undefined) => Promise<...>' is not assignable to type '<TResult1 = undefined, TResult2 = never>(onfulfilled?: ((value: undefined) => TResult1 | PromiseLike<TResult1>) | null | undefined, onrejected?: ((reason: any) => TResult2 | PromiseLike<...>) | null | undefined) => PromiseLike<...>'.
        Types of parameters 'onfulfilled' and 'onfulfilled' are incompatible.
          Types of parameters 'value' and 'value' are incompatible.
            Type 'void' is not assignable to type 'undefined'.

18     .noRepsonseEndpoint(async (req, res) => {
                           ~~~~~~~~~~~~~~~~~~~~~

test/fixtures/testAPIServer.ts:45:26 - error TS2345: Argument of type '(req: TypedRequest<Readonly<RemoveKey<EmptyInitialEndpointDefinition<"GET">, "query"> & { query: { maybeParam: string | undefined; }; }>>, res: Response<...>) => Promise<...>' is not assignable to parameter of type 'RouteHandler<Readonly<RemoveKey<EmptyInitialEndpointDefinition<"GET">, "query"> & { query: { maybeParam: string | undefined; }; }>>'.
  Type 'Promise<void>' is not assignable to type 'PromiseOrValue<undefined>'.

45     .optionalQueryParams(async (req, res) => {
                            ~~~~~~~~~~~~~~~~~~~~~

This is where I started to falldown as I am not sure what the expected behavior is from the library in this situation. Probably could have resolved from returning undefined in these test cases, but that did not feel right.

Would love to use this as an opportunity to learn more about the library, so that I might be able to start to get more involved in its maintenance.

Let me know what you think....

jacobdr avatar May 17 '20 05:05 jacobdr

Hi @jacobdr,

Thank you for spending the time to research this problem.

It appears that express has finally received some support for typed requests. As you can see I had to create a type TypedRequest because it was not possible to specify the type of a request's body or query params. But now we don't need this type anymore since express supports it out of the box with template parameters.

Doing so seems to fix the bug for me. I created #22 to test this solution. Can you confirm that this solution works for you as well?

hmil avatar May 17 '20 18:05 hmil