Skip to content
This repository has been archived by the owner on Mar 9, 2020. It is now read-only.

Fix withRequestMiddlewares signatures #134

Merged
merged 2 commits into from Jan 4, 2018
Merged

Fix withRequestMiddlewares signatures #134

merged 2 commits into from Jan 4, 2018

Conversation

gcanti
Copy link
Contributor

@gcanti gcanti commented Jan 4, 2018

@cloudify I was reading your post and noticed a possible issue: I think that the type parameter RH should be moved to the right

// current signature
declare function withRequestMiddlewares<RH, R1, R2, T1, T2>(
  m1: IRequestMiddleware<R1, T1>,
  m2: IRequestMiddleware<R2, T2>
): (handler: (v1: T1, v2: T2) => Promise<IResponse<RH>>) => RequestHandler<RH | R1 | R2>

declare const m1: IRequestMiddleware<'R1', 'T1'>
declare const m2: IRequestMiddleware<'R2', 'T2'>
declare const handler: (v1: 'T1', v2: 'T2') => Promise<IResponse<'RH'>>

// const x: (req: Request) => Promise<IResponse<{} | "R1" | "R2">>
const x = withRequestMiddlewares(m1, m2)(handler)

Note the inferred type {} for RH

// proposed signature
declare function withRequestMiddlewares2<R1, R2, T1, T2>(
  m1: IRequestMiddleware<R1, T1>,
  m2: IRequestMiddleware<R2, T2>
): <RH>(handler: (v1: T1, v2: T2) => Promise<IResponse<RH>>) => RequestHandler<RH | R1 | R2>

// const y: (req: Request) => Promise<IResponse<"R1" | "R2" | "RH">>
const y = withRequestMiddlewares2(m1, m2)(handler)

@codecov
Copy link

codecov bot commented Jan 4, 2018

Codecov Report

Merging #134 into master will increase coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #134      +/-   ##
==========================================
+ Coverage   90.07%   90.08%   +<.01%     
==========================================
  Files          54       54              
  Lines        1381     1382       +1     
  Branches      152      152              
==========================================
+ Hits         1244     1245       +1     
  Misses        137      137
Impacted Files Coverage Δ
lib/utils/request_middleware.ts 63.26% <100%> (+0.76%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update bfd6ba6...9d8ce90. Read the comment docs.

@gunzip gunzip requested a review from cloudify January 4, 2018 12:38
@cloudify
Copy link
Contributor

cloudify commented Jan 4, 2018

Thanks @gcanti ! I'll update the post too!

@cloudify cloudify merged commit 12ef45b into pagopa-archive:master Jan 4, 2018
@gcanti gcanti deleted the withRequestMiddlewares-signatures branch January 4, 2018 13:44
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
2 participants