-
-
Notifications
You must be signed in to change notification settings - Fork 17.2k
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
Decoupling express.request and express.response from express application #2812
Comments
I think this will eventually become part of https://github.com/pillarjs/extend-proto |
Well, part of it. We'll probably have a separate module for the default "express" properties. |
Seems related to #2432 ? |
I'm not sure what the state of Express 5 is, but is this something that would be up for consideration/discussion? |
I think this is a good idea, @aoberoi Especially that https://github.com/expressjs/express/blob/master/lib/router/index.js#L131-L136 |
@kevinsimper If you'll humor me, I'm curious to learn more about your use case. I'm not sure this is the best issue to discuss this, the conversation around moving req/res out is spread out in a couple of places: I'm happy to inform you work has begun on moving them to their own repos. But I don't have a use case myself where this would benefit me, so detail about how you plan to use this would be valuable! I.E, what are you doing today, why is it suboptimal, and what would you prefer to be able to do? @wesleytodd has been the champion for including this in v5, and it will likely still happen, but since I have you I'd love to hear more about what problem this could solve. Thank you ❤️ |
|
Well their included link points to |
Ah. Yes, that is a private function, as the public call to that is |
Thanks for the responses! @jonchurch I want to be able to test this code and ensure that send is being called. import express, { Request, Response } from "express";
export const app = express.Router();
app.get("/", (req: Request, res: Response) => {
res.send("Login");
}); I can not see other ways than doing this: import express from "express";
import fetch from "node-fetch";
import { app as loginRoutes } from "./login";
test("login", (done) => {
console.time("start-test-server");
const server = express();
server.use(loginRoutes);
const listener = server.listen(0);
listener.on("listening", async () => {
const address = listener.address();
if (typeof address === "string") return done(new Error("no port"));
const req = await fetch("http://localhost:" + address.port);
const data = await req.text();
expect(data).toBe("Login");
listener.close(() => {
console.timeEnd("start-test-server");
done();
});
});
}); Which is not that easy to read, it does not look clean. And the test takes on my i7 anywhere from 27 to 41 ms to run. That is slow compared to unit tests. With typescript it is not possible to create a Response object that does not contain all 79 properties of the response object. Here is an example: import express, { Response } from "express";
test("login route", () => {
const route = (req, res: Response) => {
res.send("Login");
};
const mockSend = jest.fn();
route({}, { send: mockSend });
}); It will give you this message:
I don't believe that other projects should adapt to typescript, so don't think about the typescript part. I just wanted to show that I have tried other ways, but it will not work with typescript because of the difficulties of mocking large objects. |
I wish I knew typescript so I could better help, but maybe this will help: the decoupling that this issue is talking about won't suddenly make request and reaponse prototypes usable outside of a http server, as it heavily relies on the state set up there. But if you just want to make them in order to satisfy type checks, the express module already exports them for usage (no real difference than when they will be decoupled into a different module) under |
I'd like to be able to use
express.request
andexpress.response
outside of an expressapplication
"instance". This means I'd be able to use them as prototypes for my own request and response objects. Additionally, I'd like to also define which objects they delegate to for their prototypes (I understand this last part is possible already but its not first-class and it won't work in an ES2015 world where imports are read-only).I'm a big fan of the work that was done to decouple the router implementation into its own module so that it no longer assumes the presence of an express application. I believe what I am proposing is in the spirit of that effort.
A couple use cases I'm hoping to address:
express.request
andexpress.response
interfaces. I'd like to write unit tests that are minimal, so I don't want to initialize an express application, nor do I want it to listen on a port or socket. Ideally, I could create some request and response objects directly from the express prototypes, and also plug in a "mock" forhttp.IncomingMessage
andhttp.ServerResponse
(that don't need to actually do any networking) to be one level up the prototype chain. Then I could just invoke my router'shandle()
method with the arguments I've constructed and set up assertions on the behavior. This is currently not possible because both request and response have methods that read settings fromthis.app
. I've take a look at the tests for pillarjs/router and they seem to be limited to only use API on request and response that are provided by node core.http.IncomingMessage
andhttp.ServerResponse
, we had a declared interface (one that the aforementioned types already adhere to or do so through a thin adapter) for whatexpress.request
andexpress.response
rely on up the prototype chain, we could enable much more innovation in the ecosystem. Express would also benefit from seeing experimentation on those layers and adopt proven and useful extensions back into its core.Open Questions:
The text was updated successfully, but these errors were encountered: