url: implement URL.prototype.toJSON #11236

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

+const URL = require('url').URL;
+
+// Tests below are not from WPT.
+const url = new URL('http://example.org');


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

@@ -563,6 +563,21 @@ selection of which characters to percent-encode may vary somewhat from what the
 The `toString()` method on the `URL` object returns the serialized URL. The
 value returned is equivalent to that of `url.href`.

+#### url.toJSON()
+


* Returns: {String}

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

doc/api/url.md

+#### url.toJSON()
+
+The `toJSON()` method on the `URL` object returns the serialized URL. The
+value returned is equivalent to that of `url.href`. This method is automatically


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>

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

joyeecheung added this to Landed in Spec in WHATWG URL implementation Feb 8, 2017

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

TimothyGu moved this from Landed in Spec to Done in WHATWG URL implementation Feb 17, 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

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

url: implement URL.prototype.toJSON #11236

url: implement URL.prototype.toJSON #11236

targos commented Feb 8, 2017

targos commented Feb 8, 2017

targos commented Feb 8, 2017

joyeecheung Feb 8, 2017

targos Feb 8, 2017

targos Feb 8, 2017

joyeecheung Feb 8, 2017

joyeecheung Feb 8, 2017

joyeecheung left a comment

targos commented Feb 8, 2017

TimothyGu left a comment

TimothyGu Feb 8, 2017

TimothyGu Feb 8, 2017

jasnell commented Feb 13, 2017

targos commented Feb 14, 2017

targos commented Feb 16, 2017

jasnell commented Feb 16, 2017

jasnell commented Feb 16, 2017

url: implement URL.prototype.toJSON #11236

url: implement URL.prototype.toJSON #11236

Conversation

targos commented Feb 8, 2017

Checklist

Affected core subsystem(s)

targos commented Feb 8, 2017

targos commented Feb 8, 2017

joyeecheung Feb 8, 2017

Choose a reason for hiding this comment

targos Feb 8, 2017

Choose a reason for hiding this comment

targos Feb 8, 2017

Choose a reason for hiding this comment

joyeecheung Feb 8, 2017

Choose a reason for hiding this comment

joyeecheung Feb 8, 2017

Choose a reason for hiding this comment

joyeecheung left a comment

Choose a reason for hiding this comment

targos commented Feb 8, 2017

TimothyGu left a comment

Choose a reason for hiding this comment

TimothyGu Feb 8, 2017

Choose a reason for hiding this comment

TimothyGu Feb 8, 2017

Choose a reason for hiding this comment

jasnell commented Feb 13, 2017

targos commented Feb 14, 2017

targos commented Feb 16, 2017

jasnell commented Feb 16, 2017

jasnell commented Feb 16, 2017