Skip to content

feat(mbe): add typing for typed request handlers#20

Merged
mohammadalfaiyazbitgo merged 1 commit intomasterfrom
WP-4622/handle-typed-params
Jun 10, 2025
Merged

feat(mbe): add typing for typed request handlers#20
mohammadalfaiyazbitgo merged 1 commit intomasterfrom
WP-4622/handle-typed-params

Conversation

@mohammadalfaiyazbitgo
Copy link
Copy Markdown
Contributor

Ticket: WP-4622

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

Adds TypeScript typing support for Master API request handlers and integrates those types into the shared response middleware and the generate-wallet flow.

  • Refactors withResponseHandler to use a generic typed request instead of raw Express/BitGo unions
  • Introduces MasterApiSpecRouteHandler, MasterApiSpecRouteRequest and a generic alias for routing types
  • Updates the generate-wallet handler to consume req.decoded from the typed request instead of req.body

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
src/shared/responseHandler.ts Middleware now expects GenericMasterApiSpecRouteRequest and casts Express req
src/masterBitgoExpress/routers/masterApiSpec.ts Defines new typed handler/request aliases and applies them to router.post call
src/masterBitgoExpress/generateWallet.ts Handler signature switched to use MasterApiSpecRouteRequest<'v1.wallet.generate','post'>
Comments suppressed due to low confidence (5)

src/shared/responseHandler.ts:19

  • The ServiceFunction type is now locked to MasterApiSpecRouteRequest<any, any>, removing the ability to directly handle plain Express.Request or BitGoRequest. Consider making ServiceFunction generic or restoring a union to support other request shapes.
req: MasterApiSpecRouteRequest<any, any>,

src/shared/responseHandler.ts:30

  • The middleware signature no longer includes BitGoRequest, but casts req later. You might instead type this parameter as GenericMasterApiSpecRouteRequest to avoid casting and make the handler’s contract clearer.
return async (req: Request, res: EncodedResponse, next: NextFunction) => {

src/masterBitgoExpress/routers/masterApiSpec.ts:99

  • [nitpick] The type alias MasterApiSpec shadows the constant of the same name. Consider renaming the type alias (e.g. MasterApiSpecType) to avoid confusion.
export type MasterApiSpec = typeof MasterApiSpec;

src/masterBitgoExpress/routers/masterApiSpec.ts:111

  • [nitpick] GenericMasterApiSpecRouteRequest is quite verbose. A shorter alias (e.g. AnyMasterApiRequest) might improve readability.
export type GenericMasterApiSpecRouteRequest = MasterApiSpecRouteRequest<any, any>;

src/masterBitgoExpress/routers/masterApiSpec.ts:125

  • The new typed handler is not covered by existing tests. Consider adding a unit or integration test to verify that req.decoded properties are correctly populated and handled.
withResponseHandler(async (req: GenericMasterApiSpecRouteRequest) => {

Comment on lines +30 to +32
return async (req: Request, res: EncodedResponse, next: NextFunction) => {
try {
const result = await Promise.resolve(fn(req, res, next));
const result = await fn(req as unknown as GenericMasterApiSpecRouteRequest, res, next);
Copy link

Copilot AI Jun 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Casting via unknown is a workaround; consider updating the middleware’s req type directly to GenericMasterApiSpecRouteRequest so you can call fn(req, ...) without an intermediate cast.

Copilot uses AI. Check for mistakes.
@mohammadalfaiyazbitgo mohammadalfaiyazbitgo merged commit e35bf00 into master Jun 10, 2025
3 checks passed
@mohammadalfaiyazbitgo mohammadalfaiyazbitgo deleted the WP-4622/handle-typed-params branch June 10, 2025 14:29
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

Successfully merging this pull request may close these issues.

3 participants