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

Release 2.0 #60

Open
wants to merge 29 commits into
base: master
Choose a base branch
from
Open

Release 2.0 #60

wants to merge 29 commits into from

Conversation

dougwilson
Copy link
Contributor

@dougwilson dougwilson commented Oct 14, 2017

This is a tracking issue for release 2.0.

Try the latest beta with npm install router@2.0.0-beta.1

Please keep feature requests in their own issues

If you want to make a comment on a particular change, please make the comment in the "Files changed" tab so comments are not lost during a rebase.

List of changes for release:

List of things to refactor:

  • Use standard style

Testing this release

If you want to try out this release, you can install it with the following commands:

$ npm cache clean router
$ npm install pillarjs/router#2.0

Owners/collaborators: please do not merge this PR :)

@wesleytodd
Copy link
Member

Once the other two pending PR's are done I will do the final one "Use standard style". Can we look at setting a date as a goal to get out 2.0 on this?

@dougwilson dougwilson added this to the 2.0 milestone Jul 19, 2018
@edevil
Copy link

edevil commented Sep 10, 2018

@dougwilson @wesleytodd Is there anything I can do to help move this forward?

@markstos
Copy link

I'm looking forward to the new release with promise support.

lib/layer.js Show resolved Hide resolved
@xjamundx
Copy link

xjamundx commented Dec 12, 2021

Would it be useful to explicitly test async functions?

I know there are many places where we test directly returning Promise.reject(). There are at least 2 scenarios I think that would be worth looking at:

For example all of these should be treated in the exact same way:

async function middleware1(req, res, next) {
   await Promise.reject(new Error("ok bye"));
}

async function middleware2(req, res, next) {
   throw new Error("ok bye");
}

function middleware3(req, res, next) {
   return Promise.reject(new Error("ok bye"));
}

function middleware4(req, res, next) {
   throw new Error("ok bye");
}

There are a few ways you can detect async function support if you want to do that, including some of the ones mentioned here:
https://stackoverflow.com/a/46127053/496735

Happy to file a follow on PR with these tests as well.

@hypesystem
Copy link

hypesystem commented Mar 16, 2022

Hey, if there's anything I can help with to move this forward, I would love to contribute, but don't really know how 😄 (been using express since 2014, pretty familiar with how it works, happy to write some code or contribute in any other way).

Is the checklist in the original description accurate, or is there more to do?

@wesleytodd
Copy link
Member

expressjs/express@74beeac

This commit on express is breaking when I try to merge up to 4.19.0 on the 5.x branch. I don't see it associated with any PR or additional context. For now I am going to disable the test in the express side but wanted to flag it here so we could figure out the history and re-enable before landing this.

@wesleytodd
Copy link
Member

wesleytodd commented Mar 20, 2024

expressjs/express@631ada0

This one is also looking to not be ported over and so failing the tests. Same as above, going to disable to get the branches synced up. Then I can port these here, publish a new router prerelease and then port it back to the other branch.

Just going to edit this now that there are more:

expressjs/express@708ac4c

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

Successfully merging this pull request may close these issues.

None yet

9 participants