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

Handler types are incorrectly coupled to express.js #719

Closed
2 tasks done
cdaringe opened this issue Feb 25, 2022 · 5 comments · Fixed by #730 or #882
Closed
2 tasks done

Handler types are incorrectly coupled to express.js #719

cdaringe opened this issue Feb 25, 2022 · 5 comments · Fixed by #730 or #882

Comments

@cdaringe
Copy link

cdaringe commented Feb 25, 2022

Checks

Describe the bug (be clear and concise)

TS compilation error occurs with compatible response & reply primitives.

I expect a type signature from createProxyMiddleware to yield: (req: IncomingMessage, res: http.ServerResponse, next?: ...) => void. However, the returned handler is explicitly coupled to express: (req: expresscore.Request, res: expresscore.Response), etc.

This is invalid.

When my server tries to then:

server.use((req, res, next) => myProxyMiddleware(req,res,next))`

i get a compilation error, because i'm using raw node interfaces (legit works!), but createProxyMiddleware demands express.js handlers.

Step-by-step reproduction instructions

TypeScript concern only

1. create a typescript project
2. install a non-express server, such as fastify & middie
3. try passing a `req: IncomingMessage` and `res: http.ServerResponse` into a `createProxyMiddleware(...)(req, res)`
4. observe compilation error

Expected behavior (be clear and concise)

No compilation error

How is http-proxy-middleware used in your project?

top level dependency

What http-proxy-middleware configuration are you using?

programmatic use.

What OS/version and node/version are you seeing the problem?

n/a

Additional context (optional)

n/a

@chimurai
Copy link
Owner

chimurai commented Feb 26, 2022

Hi, thanks for the report.

I'm aware of this issue.

Looking at ways to improve typing so req and res are not coupled to express. But also vice versa; Shouldn't be hardcoded to req: http.IncomingMessage, res: http.ServerResponse, you'll get the same issue for the express ecosystem. (#650)

For now you could try type the middleware with as any:

const apiProxy = createProxyMiddleware(apiProxyOptions) as any;

(Similar issue with nextjs: #713 (comment))

@cdaringe
Copy link
Author

Will you get the same issue for express? Express types extend the base node types. In the ref’d issue, I observed ClientRequest, not IncomingMessage. Nextjs also uses the basic node primitives—the next issue is disjoint, but related. I’m willing to attempt a patch with verification of compat between server libs if you’re open to reviewing it

@chimurai
Copy link
Owner

Express users will loose express specific request and response properties when node primitives are used. This would mean regression for express users.

Happy to see a good solution which'll work for different servers.

@chimurai
Copy link
Owner

chimurai commented Feb 26, 2022

Using TypeScript Generics with default values might be a solution.

export interface RequestHandler extends express.RequestHandler {
upgrade?: (req: Request, socket: net.Socket, head: any) => void;
}

Default values for req and res
req: http.IncomingMessage
res: http.ServerResponse

If you need extend req and res from your favourite server, you can configure and use it.

@chimurai
Copy link
Owner

Fixed in #730

Removal of @types/express will be investigated in separate PR

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