connect: Add NextFunction for next, fix err parameter#24883
connect: Add NextFunction for next, fix err parameter#24883RyanCavanaugh merged 1 commit intoDefinitelyTyped:masterfrom
NextFunction for next, fix err parameter#24883Conversation
| declare namespace createServer { | ||
| export type ServerHandle = HandleFunction | http.Server; | ||
|
|
||
| type NextFunction = (err?: any) => void; |
There was a problem hiding this comment.
All of these are valid in Connect:
next();
next(null);
next(new Error("Something bad happened!"));
next(404);
next(true);|
@EvanHahn Thank you for submitting this PR! 🔔 @SomaticIT - please review this PR in the next few days. Be sure to explicitly select If no reviewer appears after a week, a DefinitelyTyped maintainer will review the PR instead. |
|
After 5 days, no one has reviewed the PR 😞. A maintainer will be reviewing the PR in the next few days and will either merge it or request revisions. Thank you for your patience! |
| export type ErrorHandleFunction = (err: Error, req: http.IncomingMessage, res: http.ServerResponse, next: Function) => void; | ||
| export type NextHandleFunction = (req: http.IncomingMessage, res: http.ServerResponse, next: NextFunction) => void; | ||
| export type ErrorHandleFunction = (err: any, req: http.IncomingMessage, res: http.ServerResponse, next: NextFunction) => void; | ||
| export type HandleFunction = SimpleHandleFunction | NextHandleFunction | ErrorHandleFunction; |
There was a problem hiding this comment.
@EvanHahn I think that req in the callbacks should use a subtype of http.IncomingMessage which also has req.originalUrl: string.
In order to make middleware easier to write to be agnostic of the route, when the fn is invoked, the req.url will be altered to remove the route part (and the original will be available as req.originalUrl). For example, if fn is used at the route /foo, the request for /foo/bar will invoke fn with req.url === '/bar' and req.originalUrl === '/foo/bar'.
-- https://www.npmjs.com/package/connect#appuseroute-fn
There was a problem hiding this comment.
That looks right to me. Would you be able to make a pull request? I don't think I'll have time, but could review it.
There was a problem hiding this comment.
I've never done pull requests before... But I'll try to do it later.
There was a problem hiding this comment.
Feel free to email me at me@evanhahn.com if I can help!
npm test.)npm run lint package-name(ortscif notslint.jsonis present).If changing an existing definition:
nextdocumentationIncrease the version number in the header if appropriate.If you are making substantial changes, consider adding atslint.jsoncontaining{ "extends": "dtslint/dt.json" }.