Skip to content

connect: Add NextFunction for next, fix err parameter #24883

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.

Sign up for GitHub

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

Merged
RyanCavanaugh merged 1 commit into from
Apr 17, 2018
Merged

connect: Add NextFunction for next, fix err parameter #24883

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 17 additions & 2 deletions types/connect/connect-tests.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,20 +4,35 @@ import connect = require("connect");
const app = connect();

// log all requests
app.use((req: http.IncomingMessage, res: http.ServerResponse, next: Function) => {
app.use((req: http.IncomingMessage, res: http.ServerResponse, next: connect.NextFunction) => {
console.log(req, res);
next();
});

// "Throw" an Error
app.use((req: http.IncomingMessage, res: http.ServerResponse, next: connect.NextFunction) => {
next(new Error("Something went wrong!"));
});

// "Throw" a number
app.use((req: http.IncomingMessage, res: http.ServerResponse, next: connect.NextFunction) => {
next(404);
});

// Stop on errors
app.use((err: Error, req: http.IncomingMessage, res: http.ServerResponse, next: Function) => {
app.use((err: any, req: http.IncomingMessage, res: http.ServerResponse, next: connect.NextFunction) => {
if (err) {
return res.end(`Error: ${err}`);
}

next();
});

// Use legacy `Function` for `next` parameter.
app.use((req: http.IncomingMessage, res: http.ServerResponse, next: Function) => {
next();
});

// respond to all requests
app.use((req: http.IncomingMessage, res: http.ServerResponse) => {
res.end("Hello from Connect!\n");
Expand Down
7 changes: 5 additions & 2 deletions types/connect/index.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// Type definitions for connect v3.4.0
// Project: https://github.com/senchalabs/connect
// Definitions by: Maxime LUCE <https://github.com/SomaticIT>
// Evan Hahn <https://github.com/EvanHahn>
// Definitions: https://github.com/DefinitelyTyped/DefinitelyTyped

/// <reference types="node" />
Expand All @@ -17,9 +18,11 @@ declare function createServer(): createServer.Server;
declare namespace createServer {
export type ServerHandle = HandleFunction | http.Server;

type NextFunction = (err?: any) => void;
Copy link
Copy Markdown
Contributor

@mohsen1 mohsen1 Apr 10, 2018

Choose a reason for hiding this comment

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

isn't error null | Error?

Copy link
Copy Markdown
Contributor Author

@EvanHahn EvanHahn Apr 10, 2018

Choose a reason for hiding this comment

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

All of these are valid in Connect:

next();
next(null);
next(new Error("Something bad happened!"));
next(404);
next(true);


export type SimpleHandleFunction = (req: http.IncomingMessage, res: http.ServerResponse) => void;
export type NextHandleFunction = (req: http.IncomingMessage, res: http.ServerResponse, next: Function) => void;
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;
Copy link
Copy Markdown

@geekley geekley Nov 12, 2019

Choose a reason for hiding this comment

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

@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

Copy link
Copy Markdown
Contributor Author

@EvanHahn EvanHahn Nov 12, 2019

Choose a reason for hiding this comment

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

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.

Copy link
Copy Markdown

@geekley geekley Nov 12, 2019

Choose a reason for hiding this comment

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

I've never done pull requests before... But I'll try to do it later.

Copy link
Copy Markdown
Contributor Author

@EvanHahn EvanHahn Nov 12, 2019

Choose a reason for hiding this comment

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

Feel free to email me at me@evanhahn.com if I can help!


export interface ServerStackItem {
Expand Down