doc: expand req.query change documentation
#1127
Conversation
|
@dougwilson - got it, and added to the API doc too! |
|
Ok, but I don't think this behavior changed between 4 and 5. Can you elaborate on why you think it should be in the migration guide? Did the behavior actually change and I am just mistaken on it not? |
_includes/api/en/5x/req-query.md
Outdated
| @@ -1,7 +1,7 @@ | |||
| <h3 id='req.query'>req.query</h3> | |||
|
|
|||
| This property is an object containing a property for each query string parameter in the route. | |||
| If there is no query string, it is the empty object, `{}`. | |||
| When disabled, the `req.query` will be an empty object (`{}`). Else, the result of configured query parser. | |||
There was a problem hiding this comment.
(Sorry that the mobile view doesn't allow me to make suggested changes).
Would the wording sound better as
When
"query parser"(ed: make a link) is set to disabled, it is the empty object{}, otherwise it is the result of the configured query parser.
My thought is beginning a sentence with "Else" seems weird (not sure if there is an English rule around that though), but also that "When disabled" is not clear it refers to the query parser setting when a user just jumps to req.query docs. Same with what "configured query parser" refers to, so having that called out and a back link to it seems useful.
There was a problem hiding this comment.
@dougwilson - made those changes, mostly as is. Please have a look at the link - as I am not sure about the link target as well as the link semantics.
There was a problem hiding this comment.
as I am not sure about the link target as well as the link semantics.
The best method to know is to test locally: https://github.com/expressjs/expressjs.com#local-setup
There was a problem hiding this comment.
unfortunately it failed (consistently) for me at the setup stage (missing ruby headers while installing json module). I will check with @HarshithaKP when she comes online and try to verify the link semantics.
meanwhile let me know if you are fine with the link target
There was a problem hiding this comment.
ok, link semantics is now verified, and fixed:
- removed back-quote of href, as it was appearing in grey
- removed the import of the link target document, as it wasn't necessary
My rationale is that because we are expanding on the API doc for better clarity, it also makes sense to add wherever it is mentioned. However, removed it from the migration guide. |
dougwilson
force-pushed
the
req-query-doc-expand
branch
from
fbd375f
to
1e50c57
Compare
Refs: #1126