express.Router with with two methods on one path does not display middleswares list correctly

[tsippert]

Please check the following example to understand better what I do mean by the title of this issue.

const listEndpoints = require('express-list-endpoints')
const express = require("express");

let app = require('express')();

const userRouter = express.Router();
userRouter.get('/', [function authorize(req,res){}] , function listUsers(req,res){});
userRouter.post('/', [function authorize(req,res){}], function createUser(req,res){});
userRouter.get('/:id', [function authorize(req,res){}] , function getUserById(req,res){});

const teamRouter = express.Router();
teamRouter.get('/', [function authorize(req,res){}] , function listUsers(req,res){});
teamRouter.post('/', [function authorize(req,res){}], function createUser(req,res){});
teamRouter.get('/:id', [function authorize(req,res){}] , function getUserById(req,res){});

app.use('/users', userRouter);
app.use('/teams', teamRouter);

console.log(listEndpoints(app));

The response would be the following:

[
  {
    path: '/users',
    methods: [ 'GET', 'POST' ],
    middlewares: [ 'authorize', 'listUsers' ]
  },
  {
    path: '/users/:id',
    methods: [ 'GET' ],
    middlewares: [ 'authorize', 'getUserById' ]
  },
  {
    path: '/teams',
    methods: [ 'GET', 'POST' ],
    middlewares: [ 'authorize', 'listUsers' ]
  },
  {
    path: '/teams/:id',
    methods: [ 'GET' ],
    middlewares: [ 'authorize', 'getUserById' ]
  }
]

In the first endpoint, we have the path /users, with the methods GET and POST. That is correct.
Now, the middlewares list is incorrect: we should also have createUser added in the list.

I would have two suggestions as a possibility:

1. To have one endpoint in the list for each method (paths would be duplicated then)
2. The list of middlewares should be associated/mapped with the method that it is associated

Thank you for the time for reading this issue, and please leave any comment if you have any questions.

[tsippert]

Just as one example of what could be done:

Changing the addEndpoints method:

const addEndpoints = function (currentEndpoints, endpointsToAdd) {
  // endpointsToAdd.forEach((newEndpoint) => {
  //   const existingEndpoint = currentEndpoints.find(
  //     (item) => item.path === newEndpoint.path
  //   )
  //
  //   if (existingEndpoint !== undefined) {
  //     const newMethods = newEndpoint.methods.filter(
  //       (method) => !existingEndpoint.methods.includes(method)
  //     )
  //
  //     existingEndpoint.methods = existingEndpoint.methods.concat(newMethods)
  //   } else {
  //     currentEndpoints.push(newEndpoint)
  //   }
  // })
  currentEndpoints.push(...endpointsToAdd);
  return currentEndpoints
}

We would than have the following output:

[
  {
    path: '/users',
    methods: [ 'GET' ],
    middlewares: [ 'authorize', 'listUsers' ]
  },
  {
    path: '/users',
    methods: [ 'POST' ],
    middlewares: [ 'authorize', 'createUser' ]
  },
  {
    path: '/users/:id',
    methods: [ 'GET' ],
    middlewares: [ 'authorize', 'getUserById' ]
  },
  {
    path: '/teams',
    methods: [ 'GET' ],
    middlewares: [ 'authorize', 'listUsers' ]
  },
  {
    path: '/teams',
    methods: [ 'POST' ],
    middlewares: [ 'authorize', 'createUser' ]
  },
  {
    path: '/teams/:id',
    methods: [ 'GET' ],
    middlewares: [ 'authorize', 'getUserById' ]
  }
]

[AlbertoFdzM]

Thank for taking your time in documenting this.

I agree with you, but I'm not sure about the output format (that is something that has bugged me for quite a while). I agree that we need to change the output but it will be a breaking change.

My proposal for the structure is to use the same structure that OpenApi v3.1 uses for documenting an API (avoiding to use names or verbs for the keys in order to keep the contracts to be iterable):

[
  {
    path: '/users',
    methods: [
      {
        verb: 'post',
        middlewares: [
          {
            name: 'createUser'
          }
        ]
      }
    ] 
  }
]

[AlexTheKunz]

@AlbertoFdzM I'm very interested in getting this fixed. As it stands, this is more of a "get all routes" and not "get all endpoints" module. At least if you care about getting the list of middleware.

But this module is also downloaded a lot, and it makes sense to me to fix this instead of creating another branch.

Since this is a breaking change, moving to a v7 seems like a good move. But it seems like it should be easy enough to get done. It looks like migonos0 already created a working solution, though I've not verified it personally. If you want help adjusting the tests, I'm happy to do that.

Edit: I don't want to jump in on this if you are no longer maintaining the repo, so that's why I asked first.