-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
Document path-to-regexp migration #1316
Document path-to-regexp migration #1316
Conversation
@dougwilson what do you think about removing references to Express 3 in this migration guide?
It seems likely to me that Express 3 is not likely to be relevant to many people at this point. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Many thanks! Apart from the observation on missing router
changes, this LGTM! Let's wait for maintainers to chime in.
en/guide/migrating-5.md
Outdated
@@ -116,6 +114,14 @@ The `res.sendfile()` function has been replaced by a camel-cased version `res.se | |||
|
|||
The `app.router` object, which was removed in Express 4, has made a comeback in Express 5. In the new version, this object is a just a reference to the base Express router, unlike in Express 3, where an app had to explicitly load it. | |||
|
|||
The new version of the router contains **breaking changes** to the way paths are parsed: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These are great, but we should also include and list changes from 2.0.0-beta.1
. In a nutshell, these are:
- New
?
,*
, and+
parameter modifiers (e.g.:/users/:user+
now matches/users/1
,/users/1/2
, etc. but not/users
) - Special
*
path segment behavior removed. This means that*
is no longer a valid path and/foo/*/bar
now matches a literal'*'
. For the previous behaviour, a(.*)
matching group should be used instead. - Regular expressions can only be used in a matching group (e.g.:
/users/\\d+
is now invalid and should be written as/users/(\\d+)
).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, fantastic, I'll update the documentation.
I guess it might be worth stating that Express 5 uses path-to-regexp 6, and linking to the documentation for that. Similar to this page (see "Express uses path-to-regexp"):
http://expressjs.com/en/guide/routing.html#route-paths
Which leads a bit deeper down the rabbit hole - that page itself should possibly be updated, but is not versioned. Any suggestions on how to proceed?
I suppose it may also be worth forking http://forbeslindesay.github.io/express-route-tester/ in order to create a version for Express 5.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1 to all.
that page itself should possibly be updated, but is not versioned. Any suggestions on how to proceed?
None from my side. Core maintainers should help us here!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The migration guide has been updated with 14ee27 - @nfantone please have a look and let me know if it's okay.
@dougwilson how should we update the routing guide? If we follow the error handling guide (which is also unversioned) there's a section "Starting in Express v5..." - perhaps we could do that?
I think my preference would be to have separate documentation for version 5 versus version 4, but obviously that's a larger undertaking.
@dougwilson I realised that the migration guide is translated into other languages. Can you let me know the process for handling the translated versions? |
Argh - I changed changed the branch name in GitHub (because I used the wrong date!) and it closed this pull request. The updated pull request is here: #1317 |
Add documentation based on the updates to path-to-regexp in https://github.com/pillarjs/router/pull/102/files.
I also took the opportunity to simplify the intro and remove references to "alpha". I'm happy to revert or extract these to a separate pull request if it's more appropriate.