express icon indicating copy to clipboard operation
express copied to clipboard

Decoupling express.request and express.response from express application

Open aoberoi opened this issue 8 years ago • 11 comments

I'd like to be able to use express.request and express.response outside of an express application "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:

  • Testing for standalone router: I have a router which depends on its request and response arguments to implement the express.request and express.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" for http.IncomingMessage and http.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's handle() 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 from this.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.
  • Extensibility: I've seen a few issues on the tracker that are essentially asking for the ability to rewire request and response to expose more or different functionality. Its been noted that the design of express was to be a thin wrapper on top of the node core HTTP API, and thats where the limitations come from. If instead of directly using private API of http.IncomingMessage and http.ServerResponse, we had a declared interface (one that the aforementioned types already adhere to or do so through a thin adapter) for what express.request and express.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:

  • If response and request don't read its settings from application, where do they read it from?
  • What is the role of application when router (middleware), request, and response don't need it? Is it just a convenience for wiring things up in the "default" way?

aoberoi avatar Nov 23 '15 05:11 aoberoi

I think this will eventually become part of https://github.com/pillarjs/extend-proto

Fishrock123 avatar Feb 19 '16 17:02 Fishrock123

Well, part of it. We'll probably have a separate module for the default "express" properties.

Fishrock123 avatar Feb 19 '16 17:02 Fishrock123

Seems related to https://github.com/expressjs/express/issues/2432 ?

jokeyrhyme avatar Dec 06 '16 02:12 jokeyrhyme

I'm not sure what the state of Express 5 is, but is this something that would be up for consideration/discussion?

aoberoi avatar Dec 21 '16 12:12 aoberoi

I think this is a good idea, @aoberoi

Especially that express.Router() is a private function, which makes testing express routes with TypeScript really difficult, unless spinning up a http server

https://github.com/expressjs/express/blob/master/lib/router/index.js#L131-L136

kevinsimper avatar May 15 '20 11:05 kevinsimper

@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 ❤️

jonchurch avatar May 15 '20 18:05 jonchurch

Especially that express.Router() is a private function

express.Router() is not a private function at all. We even document it publicly all over our API docs http://expressjs.com/en/api.html

dougwilson avatar May 15 '20 18:05 dougwilson

Well their included link points to router.handle, which is labeled private in the jsdoc (not saying you're wrong, just pointing out the context)

jonchurch avatar May 15 '20 18:05 jonchurch

Well their included link points to router.handle, which is labeled private in the jsdoc

Ah. Yes, that is a private function, as the public call to that is router(req, res, next), not router.handle(req, res, next).

dougwilson avatar May 15 '20 18:05 dougwilson

Thanks for the responses! @dougwilson Did not know of router(req, res, next), I will take a look at that!

@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:

Argument of type '{ send: jest.Mock<any, any>; }' is not assignable to parameter of type 'Response<any>'. Type '{ send: Mock<any, any>; }' is missing the following properties from type 'Response<any>': status, sendStatus, links, json, and 79 more.

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.

kevinsimper avatar May 16 '20 11:05 kevinsimper

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 express.request and express.response exports.

dougwilson avatar May 17 '20 16:05 dougwilson