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

Express v5 support #29

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft

Conversation

jonchurch
Copy link
Collaborator

@jonchurch jonchurch commented Apr 8, 2020

This PR aims to address and close #28

Currently a WIP with the goal of preparing for the release of Express v5 which adds async error support to route/middleware handlers.

This package also introduces promise support for param handling re: #12, which is a feature, at the time of opening, not planned for the Express v5 release.

This PR attempts to preserve the features that users of this package enjoy, while allowing them to upgrade to Express v5.

I will get some eyes on this from Express folks before moving from draft PR to something ready to be merged 👍

index.js Outdated

if (expressVersion[0] > 4) {
// eslint-disable-next-line no-console
console.warn(`deprecated: Package express-async-errors works with version 4.x.x of Express, you are using ${expressVersion} which supports async route handlers natively`);
Copy link
Collaborator Author

@jonchurch jonchurch Apr 8, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This deprecation notice should be updated with guidance for what users of this package should do when upgrading to v5. Currently, from a user's perspective, nothing will change. They get the same features that they enjoyed while using this package under Express v4, this is done by only applying the promise support for Router.param which this package introduces, and skipping over adding promise support for middleware/route handlers, which Express v5 provides out of the box.

@jonchurch
Copy link
Collaborator Author

jonchurch commented Apr 8, 2020

There should also be an update made to the Readme here about upgrading to Express v5 which mentions that if users aren't using the promise based version of router.param then they are safe to remove this package and upgrade straight to v5.

@jonchurch
Copy link
Collaborator Author

With pillarjs/router#92 in the works, the course of this PR will likely change to make this library a no-op under v5, printing a message to let folks know that Express supports this functionality out of the box.

@rattrayalex
Copy link

For folks interested in more color on Express v5's async/await handling, @jonchurch wrote up a nice description here: expressjs/express#4256 (comment)

Thanks @jonchurch !!

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.

Package will throw under Express v5
2 participants