-
Notifications
You must be signed in to change notification settings - Fork 6
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
Comments
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']>;
} |
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']>;
} |
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 |
Thanks @jacobdr . I appreciate you taking the time to investigate this issue. |
@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.... |
Hi @jacobdr, Thank you for spending the time to research this problem. It appears that express has finally received some support for typed requests. 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? |
Copied the examples directly, I get the following error:
Stepping down some levels, the definition of:
Changing the following:
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.
The text was updated successfully, but these errors were encountered: