| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
Sorry, something went wrong.
| getSingleQueryPair(2) | ||
| ]); | ||
| console.log(params.toString()); | ||
| // Prints 'user=abc&query=first&query=second' |
There was a problem hiding this comment.
The generator examples are good but I would guess that most people probably won’t be interested in that… how about wrapping it in a <details> tag? Does that work?
Sorry, something went wrong.
There was a problem hiding this comment.
@addaleax, if that is the case I'd rather just remove it. I agree the second generator example is a bit obscure, and it is in fact only compatible with URLSearchParams, not with Map (referenced earlier). Though the first example does seem useful to me.
Sorry, something went wrong.
There was a problem hiding this comment.
I’m fine with any of those options… maybe somebody else has a stronger opinion but otherwise just go with whatever you prefer.
Sorry, something went wrong.
There was a problem hiding this comment.
Yeah I kinda can't get the merit of the second generator example at first glance. Maybe we can leave that later with a better use case as an example?
Sorry, something went wrong.
There was a problem hiding this comment.
@joyeecheung, fair.
Sorry, something went wrong.
There was a problem hiding this comment.
Great work! Left a few comments, please take a look.
Sorry, something went wrong.
| if (init instanceof URLSearchParams) { | ||
| const childParams = init[searchParams]; | ||
| this[searchParams] = childParams.slice(); | ||
| if (init === null || init === undefined) { |
There was a problem hiding this comment.
If we have init = '' as default then init can't be undefined?
Sorry, something went wrong.
There was a problem hiding this comment.
The user can do new URLSearchParams(undefined). The default is there to echo the = "" in the spec, though now I agree it's a bit pointless.
Sorry, something went wrong.
| const childParams = init[searchParams]; | ||
| this[searchParams] = childParams.slice(); | ||
| if (init === null || init === undefined) { | ||
| // record<USVString, USVString> |
There was a problem hiding this comment.
This comment can be removed I believe.
Sorry, something went wrong.
There was a problem hiding this comment.
Fixed.
Sorry, something went wrong.
|
|
||
| // URLSearchParams constructor, empty. | ||
| // URLSearchParams constructor, no arguments | ||
| params = new URLSearchParams(); |
There was a problem hiding this comment.
Do we have new URLSearchParams(null) test?
Sorry, something went wrong.
There was a problem hiding this comment.
Not yet, nor do we have a new URLSearchParams(undefined) test. Both are added.
Sorry, something went wrong.
| getSingleQueryPair(2) | ||
| ]); | ||
| console.log(params.toString()); | ||
| // Prints 'user=abc&query=first&query=second' |
There was a problem hiding this comment.
Yeah I kinda can't get the merit of the second generator example at first glance. Maybe we can leave that later with a better use case as an example?
Sorry, something went wrong.
| const params = new URLSearchParams(val.input); | ||
| let i = 0; | ||
| for (const param of params) { | ||
| assert.deepStrictEqual(param, val.output[i], |
There was a problem hiding this comment.
How about assert.deepStrictEqual(Array.from(params), val.output, ...)?
Sorry, something went wrong.
There was a problem hiding this comment.
Good idea. Fixed also.
Sorry, something went wrong.
There was a problem hiding this comment.
Woohoo! happy to see this. Left a couple of comments but otherwise LGTM
Sorry, something went wrong.
| The `URLSearchParams` object provides read and write access to the query of a | ||
| `URL`. | ||
| `URL`. It can also be used standalone with one of the four following | ||
| constructors. |
There was a problem hiding this comment.
Not a fan of starting a sentence with "It can..."... perhaps something like:
The object can also be used standalone...
Sorry, something went wrong.
| Instantiate a new `URLSearchParams` object with a query hash map. The key and | ||
| value of each property of `obj` are always coerced to strings. | ||
|
|
||
| Warning: Unlike [`querystring`][] module, duplicate keys in the form of array |
There was a problem hiding this comment.
s/^Warning:/^*Note*:/
Sorry, something went wrong.
|
CI: https://ci.nodejs.org/job/node-test-pull-request/6141/ |
Sorry, something went wrong.
Sorry, something went wrong.
PR-URL: #11060 Fixes: #10635 Ref: whatwg/url#175 Ref: web-platform-tests/wpt#4523 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
PR-URL: #11060 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
PR-URL: #11060 Ref: whatwg/url#175 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
PR-URL: nodejs#11060 Fixes: nodejs#10635 Ref: whatwg/url#175 Ref: web-platform-tests/wpt#4523 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
PR-URL: nodejs#11060 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
PR-URL: nodejs#11060 Ref: whatwg/url#175 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
|
Added backport-requested for v7.x since it doesn't look like the spec is gonna change any time soon (re #11185 (comment)). EDIT: it was backported in #12507, pending a merge there. |
Sorry, something went wrong.
There was a problem hiding this comment.
Good
Sorry, something went wrong.
| Back | FazBrowse Home | New Git URL |
In addition to URLSearchParams and USVString, support record<USVString, USVString> and sequence<sequence<USVString>> also.
Fixes: #10635
Refs: whatwg/url#175
Refs: web-platform-tests/wpt#4523
CI: https://ci.nodejs.org/job/node-test-pull-request/6089/
Checklist
Affected core subsystem(s)
url