-
Notifications
You must be signed in to change notification settings - Fork 865
fix(types): make request & response generic and remove express dependency #478
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
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,18 +1,31 @@ | ||
import * as express from 'express'; | ||
import * as http from 'http'; | ||
import * as httpProxy from 'http-proxy'; | ||
import * as net from 'net'; | ||
|
||
export interface Request extends express.Request {} | ||
export interface Response extends express.Response {} | ||
export interface Request extends http.IncomingMessage { | ||
originalUrl?: string; // in case express, connect | ||
hostname?: string; // in case express | ||
host?: string; // in case express | ||
} | ||
export interface Response extends http.ServerResponse {} | ||
|
||
export interface RequestHandler extends express.RequestHandler { | ||
export interface RequestHandler< | ||
Request extends http.IncomingMessage = http.IncomingMessage, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Making type parameters in all exported types optional to save some possibly unnecessary breaking errors like |
||
Response extends http.ServerResponse = http.ServerResponse | ||
> { | ||
(request: Request, response: Response, next?: (err: any) => void): void; | ||
upgrade?: (req: Request, socket: net.Socket, head: any) => void; | ||
} | ||
|
||
export type Filter = string | string[] | ((pathname: string, req: Request) => boolean); | ||
export type Filter<Request extends http.IncomingMessage = http.IncomingMessage> = | ||
| string | ||
| string[] | ||
| ((pathname: string, req: Request) => boolean); | ||
|
||
export interface Options extends httpProxy.ServerOptions { | ||
export interface Options< | ||
Request extends http.IncomingMessage = http.IncomingMessage, | ||
Response extends http.ServerResponse = http.ServerResponse | ||
> extends httpProxy.ServerOptions { | ||
pathRewrite?: | ||
| { [regexp: string]: string } | ||
| ((path: string, req: Request) => string) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,9 @@ | ||
import { createProxyMiddleware as middleware } from '../src'; | ||
import { createProxyMiddleware, createProxyMiddleware as middleware } from '../src'; | ||
import { Options } from '../src/types'; | ||
import * as http from 'http'; | ||
import * as express from 'express'; | ||
import * as connect from 'connect'; | ||
import * as browserSync from 'browser-sync'; | ||
|
||
describe('http-proxy-middleware TypeScript Types', () => { | ||
let options: Options; | ||
|
@@ -156,4 +160,79 @@ describe('http-proxy-middleware TypeScript Types', () => { | |
}); | ||
}); | ||
}); | ||
|
||
describe('request response inference', () => { | ||
interface FooBarRequest extends http.IncomingMessage { | ||
foo: string; | ||
} | ||
interface FooBarResponse extends http.ServerResponse { | ||
bar: number; | ||
} | ||
const fooBarUse = (handler: (request: FooBarRequest, response: FooBarResponse) => void) => {}; | ||
|
||
fooBarUse(createProxyMiddleware((_, request) => request.foo && true)); | ||
fooBarUse( | ||
createProxyMiddleware({ | ||
onError: (_, request, response) => { | ||
request.foo; | ||
response.bar; | ||
// @ts-expect-error | ||
request.params; | ||
// @ts-expect-error | ||
response.json; | ||
}, | ||
}) | ||
); | ||
}); | ||
|
||
describe('works for third-party libraries', () => { | ||
express().use( | ||
'/proxy', | ||
createProxyMiddleware({ | ||
onError: (_, request, response) => { | ||
request.params; | ||
response.json; | ||
// @ts-expect-error | ||
request.lol; | ||
// @ts-expect-error | ||
response.lol; | ||
}, | ||
}) | ||
); | ||
|
||
connect().use( | ||
'/proxy', | ||
createProxyMiddleware({ | ||
onError: (_, request, response) => { | ||
// todo, non trivial fix @ts-expect-error | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This can be fixed by sending a PR to - use(fn: HandleFunction): Server;
+ use(fn: SimpleHandleFunction): Server;
+ use(fn: NextHandleFunction): Server;
+ use(fn: ErrorHandleFunction): Server;
- use(route: string, fn: HandleFunction): Server;
+ use(route: string, fn: SimpleHandleFunction): Server;
+ use(route: string, fn: NextHandleFunction): Server;
+ use(route: string, fn: ErrorHandleFunction): Server; ...this would make the inference work. There's no other way to fix it on our end. Now the question is: Are the improvements in this PR worth having http-proxy-middleware to not work with connect properly? I'm okay with all answers although I would push "Yes" because as http-proxy-middleware should not be responsible for type gotchas of connect, nor should it's users be affected. |
||
request.params; | ||
/* | ||
problem with connect types, | ||
request somehow gets inferred to `any` because of `connect.ErrorHandleFunction` | ||
|
||
connect types are anyway pretty weird, like in `connect().use((req, res) => {})`, | ||
`req` and `req` get inferred to `any` | ||
*/ | ||
|
||
// @ts-expect-error | ||
response.json; | ||
}, | ||
}) | ||
); | ||
|
||
browserSync.create().init({ | ||
server: { | ||
middleware: [ | ||
createProxyMiddleware({ | ||
onError: (_, request, response) => { | ||
// @ts-expect-error | ||
request.params; | ||
// @ts-expect-error | ||
response.json; | ||
}, | ||
}), | ||
], | ||
}, | ||
}); | ||
}); | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Afaict
next(err)
could lead to bugs as next could beundefined