-
Notifications
You must be signed in to change notification settings - Fork 1.7k
Outstanding issues with the 5.x api docs #1126
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
Comments
I can help with few of these; the key question is: how critical those are for express 5.0? |
@dougwilson - I may be missing something; but https://github.com/expressjs/express/blob/2d519077ea6c7c52a16098c7cf6d8d667a863c7b/lib/request.js#L235 still shows the function definition; Is there a pending PR that removes this? |
This list is regarding the express 5.x code; linking to 4.x code would indeed showing it as existing, as it did exist in 4.x series :) |
Github provides a neat feature where you can browse the code for a particular version, but using the branch drop down and changing it to tag and picking the version tag of code to view. For example, the latest 5 request helpers can be viewed at the url https://github.com/expressjs/express/blob/5.0.0-alpha.8/lib/request.js . Another method is to git clone the repository and you can git checkout a specific version tag to explore the code of that version. I hope this is helpful 👍 |
@dougwilson, can you pls explain what this is? I looked at the doc of app.param(), and found that it is complete. what is missing here? |
@dougwilson, http://expressjs.com/en/guide/using-middleware.html#middleware.router has explanation for next('router'). Is this sufficient or more detail is required ?. If yes , where should I add it ? |
I'm starting work on the 5.x api reference for how to use promises in handlers/middleware. I see there are other places on the site that should reference promise based middleware and the express error handler (like the getting started docs). I'll take a stab at those and create separate PRs for each |
In the 5.x docs, the name parameter is still documented as an optional parameter, but that parameter support has been removed.
Right. This tracker is mainly about the 5.x api docs (http://expressjs.com/en/5x/api.html) not including it. All the places that are documenting |
Looking here #398 (comment), there are some good things to do for the 5.0 production release.
I believe the first item is mostly covered in Doug's list here. The last two need to be accounted. One other thing that comes to mind is that we need to make sure all links in Guide and Getting Started point to the 5.x api docs. I can create separate issues for these or perhaps add them to the tasks above? |
I think only the OP can edit the top comment, is that right? @ryhinchey if you can edit, let's just use this issue; if not, do you want to open a new issue so you can own updating th task list in it? |
for the context, the only pending item in the list is #1131 really.
I suggest we stick to the original list and not doing this at this point of time - it is not a blocker, and can be done post release as well. |
You need write perms on the repo to edit comments that aren't yours |
Alrightly, with that, and 1131 merged, I'm going to close this issue as completed. |
Uh oh!
There was an error while loading. Please reload this page.
5.x docs investigation notes:
req.host
(doc: addreq.host
property documentation #1130)req.query
should say that it is{}
when disabled, otherwise result of configured "query parser"app.router
(doc: addapp.router
documentation #1129)app.param
documents name as optional, but that whole system was removed.req.param
does not exist any longer (doc: removereq.param
API documentation #1128)next('router')
The text was updated successfully, but these errors were encountered: