feature: Allow to use app.use(createExpressRouter()) instead of useExpressServer(app)

[alcalyn]

Description

I'm integrating routing controllers in an existing application to have a better controller structure.

I have lot of controllers, so I don't want to migrate all of them for now.

You fixed an issue where two controllers in routing-controllers were called here: #220

But it is still calling express controller after a annotated controller from routing-controllers:

    @Get('/api/users')
    getAll()
    {
        return userReporitory.findAll();
    }

    app.get('/**', async (_, res) => {
        res.render('page.ejs'); // render main app html page for html5 navigation
    });

Both are called. It was not the case before, and it works when I put the second controller in a routing-controller class.

I found this workaround:

    useExpressServer(app, {...});

    // If an api endpoint has already been called, stop routing chain
    app.all('/**', (req, res, next) => {
        if (!res.headersSent) {
            next();
        }
    });

    // other express routers

    app.get('/**', async (_, res) => {
        res.render('page.ejs'); // render main app html page for html5 navigation
    });

Proposed solution

When I started integrating this library, I found we can do either app = createExpressServer and useExpressServer(app).

But I expected we can do something like app.use(createExpressRouter({ controllers: [...] })).

So that we can gradually migrate routers one by one:

    app.use(staticsRouter());

     // Inside myApiRouter(), I can use either routing-controller, like return createExpressRouter(...)
    // or normal express like const router = Router(); ... ; return router;
    app.use(myApiRouter());

    app.use(pwaRouter());
    app.use(seoRouter());
    app.use(pagesRouter());

And this way we have an independant router than can be used among other express routers.

[attilaorosz]

I use a similar approach at work but I’m just binding to .get(‘*’, (req, res) => … for the default page rendering after setting up routing-controllers. I haven’t encountered an issue where it would call through. Could you setup a quick repro repo to test this?

[alcalyn]

Sure:

app.ts

import { Get, JsonController, createExpressServer } from 'routing-controllers';
import { Express } from 'express';

@JsonController()
class Controller
{
    @Get('/api/users')
    getUsers() {
        console.log('Api controller called');

        return [
            { pseudo: 'Hello' },
        ];
    }
}

const app: Express = createExpressServer({
    controllers: [
        Controller
    ],
});

app.get('/**', (_, res) => {
    console.log('Default controller called');

    res.status(200).send('Hello default').end();
});

app.listen(3030, () => {
    console.log('Listening on port 3030...');
});

tsconfig.json

{
  "compilerOptions": {
    "target": "es2016",
    "experimentalDecorators": true,
    "emitDecoratorMetadata": true,
    "module": "commonjs",
    "esModuleInterop": true,
    "forceConsistentCasingInFileNames": true,
    "strict": true,
    "skipLibCheck": true
  }
}

Run:

# dependencies
yarn add @types/express@^4.17.21 @types/node@^20.11.20 class-transformer@^0.5.1 class-validator@^0.14.1 express@^4.18.2 routing-controllers@^0.10.4 ts-node@^10.9.2 typescript@^5.3.3

# start app
yarn ts-node app.ts

# make api call
curl http://localhost:3030/api/users

Curl result is expected: [{"pseudo":"Hello"}]

But I see default controller has been called also, see server logs:

Api controller called
Default controller called
Error: Cannot set headers after they are sent to the client
    at ServerResponse.setHeader (node:_http_outgoing:652:11)
    at ServerResponse.header (/home/lequipe/dev/reproducer/node_modules/express/lib/response.js:794:10)
    at ServerResponse.send (/home/lequipe/dev/reproducer/node_modules/express/lib/response.js:174:12)
    at /home/lequipe/dev/reproducer/app.ts:26:21
    at Layer.handle [as handle_request] (/home/lequipe/dev/reproducer/node_modules/express/lib/router/layer.js:95:5)
    at next (/home/lequipe/dev/reproducer/node_modules/express/lib/router/route.js:144:13)
    at Route.dispatch (/home/lequipe/dev/reproducer/node_modules/express/lib/router/route.js:114:3)
    at Layer.handle [as handle_request] (/home/lequipe/dev/reproducer/node_modules/express/lib/router/layer.js:95:5)
    at /home/lequipe/dev/reproducer/node_modules/express/lib/router/index.js:284:15
    at param (/home/lequipe/dev/reproducer/node_modules/express/lib/router/index.js:365:14)

[attilaorosz]

The main issue is that we have to call next after the controller otherwise the OnAfter middlewares would not run. Personally I would remove the OnAfter middleware support completely because if you need something like that then you are better off with an interceptor anyway.

But unfortunately here we are with that legacy stuff.

Your workaround is the best bet for now, even if we provide a createExpressRouter it would contain all additionals (middlewares, interceptors, so we can have to call next) and the execution would be the same.

I'm going to keep this open because this is something we have to address.

[attilaorosz]

Another solution could be support for a default controller which would be triggered when no routes could be matched. That way you could define the default page renderer. It would complicate things with routePrefix tho.

[alcalyn]

In my use case, I don't only have a default /** route, I have a router before, and some routers after (containing the default route).

Your workaround is the best bet for now, even if we provide a createExpressRouter it would contain all additionals (middlewares, interceptors, so we can have to call next) and the execution would be the same.

I don't know how routing controllers work internally, nor used interceptors yet. I thought that an express router was independant from others routers, so that's why I believe that a createExpressRouter could solve it, like:

express app
    - router A
        - middleware
        - route
        - route
    - createExpressRouter
    - router B
        - middleware
        - route
    - router C

[attilaorosz]

Tho they are independent from other middlewares, calling next propagates.

Internally we are not using routers but it would not make much difference.