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

querystring: fix stringify for empty array #33918

Closed
wants to merge 1 commit into from

Conversation

sapics
Copy link
Contributor

@sapics sapics commented Jun 17, 2020

Fixes #33910

Currently
querystring.stringify({a:1, b:[]}) return "a=1&"
and
querystring.stringify({a:1}) return "a=1"
and
querystring.stringify({a:[], b:[]}) return ""

If this PR is merged, first one will change to
querystring.stringify({a:1, b:[]}) return "a=1".

I think that this is better for consistency.

Checklist

@nodejs-github-bot nodejs-github-bot added the querystring Issues and PRs related to the built-in querystring module. label Jun 17, 2020
@sapics sapics force-pushed the lib/fix-querystring-empty-array branch 4 times, most recently from f194c8a to e334dbe Compare June 17, 2020 05:49
@addaleax
Copy link
Member

For whoever lands this: The commit message should start with querystring:

@addaleax addaleax added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Jun 19, 2020
@nodejs-github-bot
Copy link
Collaborator

@sapics sapics force-pushed the lib/fix-querystring-empty-array branch from e334dbe to a117ec5 Compare June 19, 2020 17:11
@sapics
Copy link
Contributor Author

sapics commented Jun 19, 2020

@addaleax Thanks for noticing! I replace commit message from "lib: fix querystring.stringify for empty array" to "querystring: fix stringify for empty array", and update title also.

@sapics sapics changed the title lib: fix querystring.stringify for empty array querystring: fix stringify for empty array Jun 19, 2020
@nodejs-github-bot
Copy link
Collaborator

jasnell pushed a commit that referenced this pull request Jun 24, 2020
PR-URL: #33918
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
@jasnell
Copy link
Member

jasnell commented Jun 24, 2020

Landed in d4a1c98

@jasnell jasnell closed this Jun 24, 2020
@sapics sapics deleted the lib/fix-querystring-empty-array branch June 24, 2020 23:35
codebytere pushed a commit that referenced this pull request Jun 27, 2020
PR-URL: #33918
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
@codebytere codebytere mentioned this pull request Jun 28, 2020
codebytere pushed a commit that referenced this pull request Jun 30, 2020
PR-URL: #33918
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
codebytere pushed a commit that referenced this pull request Jul 10, 2020
PR-URL: #33918
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
@codebytere codebytere mentioned this pull request Jul 13, 2020
codebytere pushed a commit that referenced this pull request Jul 14, 2020
PR-URL: #33918
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. querystring Issues and PRs related to the built-in querystring module.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

querystring inconsistent behaviour
4 participants