Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support async middleware chains #146

Closed
UncleClapton opened this issue Jun 27, 2021 · 2 comments · Fixed by #196 · May be fixed by #148
Closed

Support async middleware chains #146

UncleClapton opened this issue Jun 27, 2021 · 2 comments · Fixed by #196 · May be fixed by #148

Comments

@UncleClapton
Copy link

UncleClapton commented Jun 27, 2021

Hello! I'm trying to implement a response time calculator in a Next API Route package I'm developing, and running into a roadblock.

The middleware stack currently runs synchronously, which prevents middleware from executing code after an async handler has been resolved. Instead, the stack unravels after an awaited call, executing code after next() in a middleware while doing so. The example below illustrates this.

Would there be any interest in a PR which changes this behavior and adds accompanying documentation?

The simplest method would be to change next() to return the result of it's expression. This would let users form async middleware chains in their applications, resolving the issue described above. It also has the benefit of being a non-breaking change.

The only major limitation is that async middleware would be all or nothing. Any middleware that fails to await or return the result of next() would break the async chain. This could cause some major confusion, and would need to be well documented. The same behavior can be found in other libraries such as koa.js and the long awaited Express v5, however, so it's not an entirely new concept.

Example

import nc from 'next-connect';
import { someAsyncWork } from './another-module';

const middleware = (req, res, next) => {
  console.log('1');

  await next(); // await has no bearing in the current version, however with the proposed solution, it would.

  console.log('4');
};

const handler = async (req, res) => {
  console.log('2');

  const someData = await someAsyncWork();
  res.status(200).json(someData);

  console.log('3');
};

export default nc()
  .use(middleware)
  .get('/', handler);

Output

Expected Actual
1 1
2 2
3 4
4 3
@hoangvvo
Copy link
Owner

hoangvvo commented Jul 4, 2021

Yeah I think this would be a simple change. Would you like to submit a PR for it?

@UncleClapton
Copy link
Author

Sure, I'll get to work on it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants