Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Example definition in README fails to compile in Typescript strict mode #19

Open
jacobdr opened this issue May 9, 2020 · 6 comments
Open

Comments

@jacobdr
Copy link

jacobdr commented May 9, 2020

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
Copy link
Author

jacobdr commented May 9, 2020

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
Copy link
Author

jacobdr commented May 10, 2020

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
Copy link
Author

jacobdr commented May 10, 2020

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

@hmil
Copy link
Owner

hmil commented May 13, 2020

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

@jacobdr
Copy link
Author

jacobdr commented May 17, 2020

@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: 35ed981

After that, I tried to implement a fix. That work was here: bcdba81

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....

@hmil
Copy link
Owner

hmil commented May 17, 2020

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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants