refactor(#1136): replace http-proxy w/ httpxy#1160
refactor(#1136): replace http-proxy w/ httpxy#1160SukkaW wants to merge 8 commits intochimurai:masterfrom
http-proxy w/ httpxy#1160Conversation
|
Hi @SukkaW Thanks for the PR! Didn't know about Did a quick check and looks really nice so far. Most of the types are working. Found a small issue in the typing where
Added an extra check to prevent 🔧 Can you rebase with master? http-proxy-middleware/test/types.spec.ts Lines 393 to 401 in 6436ffc |
| "strict": true, | ||
| "noImplicitAny": false | ||
| "noImplicitAny": false, | ||
| "skipLibCheck": true |
There was a problem hiding this comment.
Why was skipLibCheck: true needed?
There was a problem hiding this comment.
httpxy's types rely on a default export on node:http (in its .d.ts it uses import http from 'node:http').
But this only works with moduleResolution: node16/nodenext/bundler + module: node16/nodenext/preserve setup, not with moduleResolution: node + module: commonjs setup.
So rather than changing the moduleResolution and module (which will most likely result in a huge breaking change), I just enabled skipLibCheck to avoid that.
The After |
b1801dd to
c10521e
Compare
| type Interceptor<TReq = http.IncomingMessage, TRes = http.ServerResponse> = ( | ||
| buffer: Buffer, | ||
| proxyRes: TReq, | ||
| proxyRes: http.IncomingMessage, |
There was a problem hiding this comment.
proxyRes comes from the http client instead of the call site (express.js), thus this should not be TReq.
| if (!req || !res) { | ||
| throw err; // "Error: Must provide a proper URL as target" | ||
| } |
There was a problem hiding this comment.
Technically, req and res should either be both specified or both nullish.
However, currently, httpxy can't implement this kind of overload due to how NodeJS.EventEmitter types its event arguments via a generic (which ProxyServer is extending from), so the && is replaced with || to make the TypeScript happy down the road.
|
@chimurai Also, I am currently actively working on httpxy's HTTP/2 listener support (unjs/httpxy#38). Update: almost done unjs/httpxy#102 |
|
@chimurai Hello, what else is required for the PR to merge? All TypeScript error has been fixed and the lint is now happy. |






Description
Another attempt to fix #1136.
Unlike #1159, instead of replacing
http-proxyw/http-proxy-3, the PR chooseshttpxy, because:httpxyis way more popular and more battle-tested:http-proxy-3only has about 45K weekly downloads right nowhttpxyhas about 1.1M weekly downloads right nowhttpxyhas a smaller installation size, hence a smaller footprint:http-proxy-3has 2 dependencies, its installation size is about 185 KiB (https://pkg-size.dev/http-proxy-3)httpxyhas 0 dependency (!) and its installation size is only 57 KiB (https://pkg-size.dev/httpxy)http-proxy-3claims it is used by the Vite,httpxyis actually widely used by the Nuxt.js community, and is actually getting contributions from the Vite.js Team (refactor!: code improvements unjs/httpxy#78)!Motivation and Context
See #1136
How has this been tested?
Unlike #1159, I actually get all the tests to pass and TypeScript happy! You can check the action log here: SukkaW#1
Types of changes
Checklist: