Router Spread Middleware Execution Order Inconsistency

[RohinBhargava]

I am observing some unexpected behavior when using specific route middlewares in a spread style pattern in a Router:

Scenario 1:

Consider the following code:

const appMiddleware = (req: any, res: any, next: () => void) => {
  console.log('should be line 1');
  next();
}

const routerMiddleware = (req: any, res: any, next: () => void) => {
  console.log('should be line 2');
  next();
}

const specificMiddleware = (req: any, res: any, next: () => void) => {
  console.log('should be line 3');
  next();
}

const handlerFunction = (req: any, res: { send: (arg0: string) => void; }) => {
  console.log('should be line 4')
  res.send('finish');
}

const app = HyperExpress.express();
app.use(appMiddleware)

const testRouter = new HyperExpress.Router();
testRouter.use(routerMiddleware);
testRouter.get('/', specificMiddleware, handlerFunction);
app.use('/test', testRouter);

app.listen(8000, () => {
  console.log('server is running');
});

When calling /test, this results in:

server is running
should be line 1
should be line 3
should be line 2
should be line 4

Scenario 2:

Now if we use the code:

const app = HyperExpress.express();
app.use(appMiddleware)

const testRouter = new HyperExpress.Router();
testRouter.use(routerMiddleware);
testRouter.use(specificMiddleware);
testRouter.get('/', handlerFunction);
app.use('/test', testRouter);

app.listen(8000, () => {
  console.log('server is running');
});

The above code outputs the correct execution order:

server is running
should be line 1
should be line 2
should be line 3
should be line 4

Analysis:

I believe this is due to the compile() function on app startup. When using the spread style arguments in scenario 1, the execution order before and after sorting middlewares.sort((a, b) => a.id - b.id); is:

before sort [
  {
    id: 0,
    pattern: '',
    handler: [Function: appMiddleware],
    match: true
  },
  {
    id: 2,
    pattern: '/test',
    handler: [Function: routerMiddleware],
    match: true
  },
  {
    id: 1,
    pattern: '/test',
    handler: [Function: specificMiddleware],
    match: false
  }
]
after sort [
  {
    id: 0,
    pattern: '',
    handler: [Function: appMiddleware],
    match: true
  },
  {
    id: 1,
    pattern: '/test',
    handler: [Function: specificMiddleware],
    match: false
  },
  {
    id: 2,
    pattern: '/test',
    handler: [Function: routerMiddleware],
    match: true
  }
]

In scenario 2, the before/after looks correct:

before sort [
  {
    id: 0,
    pattern: '',
    handler: [Function: appMiddleware],
    match: true
  },
  {
    id: 2,
    pattern: '/test',
    handler: [Function: routerMiddleware],
    match: true
  },
  {
    id: 3,
    pattern: '/test',
    handler: [Function: specificMiddleware],
    match: true
  }
]
after sort [
  {
    id: 0,
    pattern: '',
    handler: [Function: appMiddleware],
    match: true
  },
  {
    id: 2,
    pattern: '/test',
    handler: [Function: routerMiddleware],
    match: true
  },
  {
    id: 3,
    pattern: '/test',
    handler: [Function: specificMiddleware],
    match: true
  }
]

I believe this is happening because when compiling a route, the line id: this.id picks up the id from the Server, which is behind the app_middlewares (?). A patch that works for me is to calculate the max id from the global middlewares and then add that to the id in the specific middlewares section of the compile function. E.g.:

const offset = middlewares.reduce((max, middleware) => middleware.id > max ? middleware.id : max, 0);

// Push the route-specific middlewares to the array at the end
if (Array.isArray(this.options.middlewares))
    this.options.middlewares.forEach((middleware) =>
        middlewares.push({
            id: this.id + offset,
            pattern,
            handler: middleware,
            match: false, // Route-specific middlewares do not need to be matched
        })
    );