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

Update applyMiddleware to return #1

Merged
merged 2 commits into from
Apr 21, 2019
Merged

Update applyMiddleware to return #1

merged 2 commits into from
Apr 21, 2019

Conversation

nahtnam
Copy link
Contributor

@nahtnam nahtnam commented Apr 17, 2019

Hello!

I noticed a bug in this library where if you use an async route which returns, it does not finish the request. For example, using applyMiddeware with this function does not work:

module.exports = async (req, res) => {
  return 'Ready!'
}

However, this does:

module.exports = (req, res) => {
  res.end('Ready!')
}

Adding a return will ensure that both methods will work.

@mhamann
Copy link
Owner

mhamann commented Apr 20, 2019

Hi! Thanks for the PR. Good catch--definitely want to merge this in. Any chance you could write / modify a quick test to prevent this from potentially regressing in the future?

@nahtnam
Copy link
Contributor Author

nahtnam commented Apr 20, 2019

@mhamann Added, let me know if you want me to change anything

nahtnam added 2 commits April 20, 2019 23:21
Hello!

I noticed a bug in this library where if you use an async route which returns, it does not finish the request. For example, using `applyMiddeware` with this function does not work:

```
module.exports = async (req, res) => {
  return 'Ready!'
}
```

However, this does:
```
module.exports = (req, res) => {
  res.end('Ready!')
}
```

Adding a `return` will ensure that both methods will work.
@mhamann mhamann merged commit 823f426 into mhamann:master Apr 21, 2019
@mhamann
Copy link
Owner

mhamann commented Apr 21, 2019

Merged and published as v1.2.0.

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 this pull request may close these issues.

2 participants