Enhance forRoutes RouteInfo object to specify API versions

[NitnekB]

Is there an existing issue that is already proposing this?

• I have searched the existing issues

Is your feature request related to a problem? Please describe it

I currently use NestJS versioning feature, which is nice BTW 🚀

I also use a custom middleware the most common way:

consumer
      .apply(CustomMiddleware)
      .forRoutes(PricesController, ProductsController);

But this caused a regression and my middleware doesn't works anymore.

The only way to make it works is to specify each route, or exclude the one we doesn't want:

consumer
      .apply(CustomMiddleware)
      .exclude({ path: '/v0/products', method: RequestMethod.POST })
      .forRoutes({ path: '/*', method: RequestMethod.ALL });

This works well, but we have to fill in the version for each route: /v0/products, ...

Describe the solution you'd like

I dont' know if we should consider this as a bug, but I think we could use another parameter versions when declaring routes on forRoutes method enhancing RouteInfo type.

Teachability, documentation, adoption, migration strategy

Here an example of what I'm thinking about:

consumer
      .apply(CustomMiddleware)
      .exclude({ path: '/products', method: RequestMethod.POST, versions: ['0', '1'] })
      .forRoutes({ path: '/*', method: RequestMethod.ALL });

What is the motivation / use case for changing the behavior?

Prevent lots of works introducing or deprecating API versions

// products.controller.ts

export const PRODUCTS_API_VERSIONS = ['0', '1'];

@Controller({ path: 'products', version: PRODUCTS_API_VERSIONS })
export class ProductsController {
}

// app.module.ts

consumer
      .apply(CustomMiddleware)
      .exclude({ path: '/products', method: RequestMethod.POST, versions: PRODUCTS_API_VERSIONS })
      .forRoutes({ path: '/*', method: RequestMethod.ALL });

[micalevisk]

I believe this feat will help on making HTTP versioning a first-class citizen on NestJS. That's cool.

---

@NitnekB As we can use multiple versions on version/defaultVersion field already, wouldn't be better to use the singular here as well?

.exclude({ ..., version: '1' })

.exclude({ ..., version: ['1'] })

[NitnekB]

@micalevisk Seems legit to me 👍

[Cyril-zip]

May I ask for any update on this issue?

[micalevisk]

@chanpaul1234572 I didn't have time to try to implement this :/

PRs are more than welcomed

[princechauhan1992]

@kamilmysliwiec I would like to implement this.

[micalevisk]

@princechauhan1992 PRs are more than welcomed 😸

[ologbonowiwi]

Guys, I will get this one! 😁

[thiagomini]

I'm taking this one, @kamilmysliwiec. I've been already able to reproduce the issue in an integration test.

[kamilmysliwiec]

Sounds great @thiagomini!

[mareksuscak]

I couldn't find the original PR that introduced version-aware middleware but the implementation is currently buggy when global prefix is configured.

It produced:

/v1/api/users/:userId

instead of:

/api/v1/users/:userId

I.e. the version number is prepended before the global prefix, not the other way around. I'll create an issue.

[thiagomini]

Hey @mareksuscak, this is the PR that introduced that. Feel free to add the necessary fix to it! I can work on that as well as soon as I have the time. Thanks for lettings us know 😊

[thiagomini]

My approach to resolving this issue would be to reproduce the same problem in this test. As soon as you get the same error, work on your way down the layers to make it pass with the least amount of changes. Then, you can refactor it to a cleaner solution.

[mareksuscak]

Here's the ticket: #10566