url: implement URL.prototype.toJSON by targos · Pull Request #11236 · nodejs/node

targos · 2017-02-08T10:11:48Z

Refs: whatwg/url#229

Checklist

make -j4 test (UNIX), or vcbuild test (Windows) passes
tests and/or benchmarks are included
documentation is changed or added
commit message follows commit guidelines

Affected core subsystem(s)

url

targos · 2017-02-08T10:14:26Z

Sorry, make test does not pass. I'm fixing it...

targos · 2017-02-08T10:19:13Z

Fixed. CI: https://ci.nodejs.org/job/node-test-pull-request/6281/

joyeecheung · 2017-02-08T10:21:09Z

test/parallel/test-whatwg-url-tojson.js

Can you port the WPT tests here like e.g. what test-whatwg-url-searchparams-set.js does for urlsearchparams-set.html? Thanks!

Is there a tool to do that?

I'm asking because it seems complicated to keep the tests up to date with WPT tests if we have to copy/paste them by hand.

Have to copy/paste at the moment, though I think a tool would definitely helpful.

I think we don't have to update a file as long as the upstream doesn't update a file(at least if it doesn't update the <script></script> part. The hope is this way we can see how up-to-date we are with the upstream tests without doing mental conversions from the WPT tests suite to Node.js core test suites.

Also this should be easier if we are updating a file instead of adding a new one(no need to do the conversion, just copy-paste)

joyeecheung

LGTM with the WPT tests ported and CI passed

targos · 2017-02-08T12:22:15Z

Updated to use WPT. New CI: https://ci.nodejs.org/job/node-test-pull-request/6283/

TimothyGu

LGTM besides some nits.

TimothyGu · 2017-02-08T19:23:10Z

doc/api/url.md

* Returns: {String}

TimothyGu · 2017-02-08T19:24:11Z

doc/api/url.md

If you think it's a good idea, you can mention url.toString() as well.

jasnell · 2017-02-13T19:16:38Z

@targos ... PTAL at @TimothyGu's comments

targos · 2017-02-14T10:23:08Z

I'll handle it tomorrow

Refs: whatwg/url#229

targos · 2017-02-16T08:15:23Z

Updated the documentation. PTAL

jasnell · 2017-02-16T19:17:10Z

CI: https://ci.nodejs.org/job/node-test-pull-request/6447/

PR-URL: #11236 Ref: whatwg/url#229 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Timothy Gu <timothygu99@gmail.com>

jasnell · 2017-02-16T20:37:28Z

Landed in 66e2630

PR-URL: nodejs#11236 Ref: whatwg/url#229 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Timothy Gu <timothygu99@gmail.com>

PR-URL: #11236 Ref: whatwg/url#229 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Timothy Gu <timothygu99@gmail.com>

Notables changes: * child_process: spawnSync() exit code now is null when the child is killed via signal (cjihrig) [nodejs#11288](nodejs#11288) * http: new functions to access the headers for an outgoing HTTP message (Brian White) [nodejs#11562](nodejs#11562) * lib: deprecate node --debug at runtime (Josh Gavant) [nodejs#11275](nodejs#11275) * tls: new tls.TLSSocket() supports sec ctx options (Sam Roberts) [nodejs#11005](nodejs#11005) * url: adding URL.prototype.toJSON support (Michaël Zasso) [nodejs#11236](nodejs#11236)

Notables changes: * child_process: spawnSync() exit code now is null when the child is killed via signal (cjihrig) [nodejs#11288](nodejs#11288) * http: new functions to access the headers for an outgoing HTTP message (Brian White) [nodejs#11562](nodejs#11562) * lib: deprecate node --debug at runtime (Josh Gavant) [nodejs#11275](nodejs#11275) * tls: new tls.TLSSocket() supports sec ctx options (Sam Roberts) [nodejs#11005](nodejs#11005) * url: adding URL.prototype.toJSON support (Michaël Zasso) [nodejs#11236](nodejs#11236) * doc: items in the API documentation may now have changelogs (Anna Henningsen) [nodejs#11489](nodejs#11489) PR-URL: nodejs#11553

Notables changes: * child_process: spawnSync() exit code now is null when the child is killed via signal (cjihrig) [nodejs#11288](nodejs#11288) * http: new functions to access the headers for an outgoing HTTP message (Brian White) [nodejs#11562](nodejs#11562) * lib: deprecate node --debug at runtime (Josh Gavant) [nodejs#11275](nodejs#11275) * tls: new tls.TLSSocket() supports sec ctx options (Sam Roberts) [nodejs#11005](nodejs#11005) * url: adding URL.prototype.toJSON support (Michaël Zasso) [nodejs#11236](nodejs#11236) * doc: items in the API documentation may now have changelogs (Anna Henningsen) [nodejs#11489](nodejs#11489) * crypto: adding support for OPENSSL_CONF again (Sam Roberts) [nodejs#11006](nodejs#11006) * src: adding support for trace-event tracing (misterpoe) [nodejs#11106](nodejs#11106) PR-URL: nodejs#11553

Notables changes: * child_process: spawnSync() exit code now is null when the child is killed via signal (cjihrig) [#11288](nodejs/node#11288) * http: new functions to access the headers for an outgoing HTTP message (Brian White) [#11562](nodejs/node#11562) * lib: deprecate node --debug at runtime (Josh Gavant) [#11275](nodejs/node#11275) * tls: new tls.TLSSocket() supports sec ctx options (Sam Roberts) [#11005](nodejs/node#11005) * url: adding URL.prototype.toJSON support (Michaël Zasso) [#11236](nodejs/node#11236) * doc: items in the API documentation may now have changelogs (Anna Henningsen) [#11489](nodejs/node#11489) * crypto: adding support for OPENSSL_CONF again (Sam Roberts) [#11006](nodejs/node#11006) * src: adding support for trace-event tracing (misterpoe) [#11106](nodejs/node#11106) PR-URL: nodejs/node#11553 Signed-off-by: Ilkka Myller <ilkka.myller@nodefield.com>

PR-URL: #55818 Fixes: #55806 Refs: #11236 Refs: #17365 Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>

PR-URL: nodejs#55818 Fixes: nodejs#55806 Refs: nodejs#11236 Refs: nodejs#17365 Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>

PR-URL: #55818 Fixes: #55806 Refs: #11236 Refs: #17365 Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>

nodejs-github-bot added dont-land-on-v4.x whatwg-url Issues and PRs related to the WHATWG URL implementation. labels Feb 8, 2017

addaleax approved these changes Feb 8, 2017

View reviewed changes

targos force-pushed the url-tojson branch from 5afa2ae to 3dc1452 Compare February 8, 2017 10:18

joyeecheung reviewed Feb 8, 2017

View reviewed changes

joyeecheung approved these changes Feb 8, 2017

View reviewed changes

targos force-pushed the url-tojson branch 2 times, most recently from 117dbe1 to 8342d2d Compare February 8, 2017 12:16

cjihrig approved these changes Feb 8, 2017

View reviewed changes

jasnell approved these changes Feb 8, 2017

View reviewed changes

TimothyGu approved these changes Feb 8, 2017

View reviewed changes

url: implement URL.prototype.toJSON

531656d

Refs: whatwg/url#229

targos force-pushed the url-tojson branch from 8342d2d to 531656d Compare February 16, 2017 08:14

TimothyGu approved these changes Feb 16, 2017

View reviewed changes

jasnell closed this Feb 16, 2017

italoacasas mentioned this pull request Feb 25, 2017

7.7.0 Proposal #11553

Merged

targos deleted the url-tojson branch June 1, 2017 08:02

This was referenced Nov 10, 2024

Update nodejs data for URL mdn/browser-compat-data#24997

Merged

Missing version info about some members of URL class & URLSearchParams class #55806

Closed

aduh95 mentioned this pull request Nov 11, 2024

doc: fix history info for URL.prototype.toJSON #55818

Merged

Uh oh!

Conversation

targos commented Feb 8, 2017

Checklist

Affected core subsystem(s)

Uh oh!

targos commented Feb 8, 2017

Uh oh!

targos commented Feb 8, 2017

Uh oh!

joyeecheung Feb 8, 2017

Choose a reason for hiding this comment

Uh oh!

targos Feb 8, 2017

Choose a reason for hiding this comment

Uh oh!

targos Feb 8, 2017

Choose a reason for hiding this comment

Uh oh!

joyeecheung Feb 8, 2017

Choose a reason for hiding this comment

Uh oh!

joyeecheung Feb 8, 2017

Choose a reason for hiding this comment

Uh oh!

joyeecheung left a comment

Choose a reason for hiding this comment

Uh oh!

targos commented Feb 8, 2017

Uh oh!

TimothyGu left a comment

Choose a reason for hiding this comment

Uh oh!

TimothyGu Feb 8, 2017

Choose a reason for hiding this comment

Uh oh!

TimothyGu Feb 8, 2017

Choose a reason for hiding this comment

Uh oh!

jasnell commented Feb 13, 2017

Uh oh!

targos commented Feb 14, 2017

Uh oh!

targos commented Feb 16, 2017

Uh oh!

jasnell commented Feb 16, 2017

Uh oh!

jasnell commented Feb 16, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants