GeezerVison

@nodejs/collaborators

Perhaps we could raise interest in this by providing examples of failing tests.

@seishun ... https://github.com/nodejs/node/blob/master/test/known_issues/test-url-parse-conformance.js :-)

We currently fail somewhere around 140+ of the test cases in the WhatWG set.

+1

Considering that the browser are exposing the global I think it makes a whole bunch of sense. I'd be interested in helping out with this. Have you broken ground on implementation @jasnell?

Can we borrow from implementations at all?

@thealphanerd ... yeah, I've got it mostly implemented already. The next step is to start running it through it's paces with tests and benchmarks and to find ways of optimizing the implementation. It's currently quite a bit slower than the existing require('url') module parsing.

Regarding borrowing from other impls, it's entirely possible that we could borrow from chromes implementation. I'm not sure yet if theirs is a pure JS impl or not. I'll look into that.

Still considering those. They are easy enough to implement given the
punycode module but I'm not sure how extensively they're used.
On Jun 1, 2016 9:40 AM, "Yorkie Liu" [email protected] wrote:

@jasnell https://github.com/jasnell How about the following 2 static
methods which is defined at IDL:

• domainToASCII(domain)
• domainToUnicode(domain)

I didn't see those in the proposal :-(

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#28 (comment), or mute
the thread
https://github.com/notifications/unsubscribe/AAa2eVA3NG0j4lEZnCUSro0RREhVL5wBks5qHbXqgaJpZM4IrtaC
.

There will also be other differences. For instance, I'm not sure if we need
the host parsing component.
On Jun 1, 2016 9:42 AM, "James M Snell" [email protected] wrote:

Still considering those. They are easy enough to implement given the
punycode module but I'm not sure how extensively they're used.
On Jun 1, 2016 9:40 AM, "Yorkie Liu" [email protected] wrote:

@jasnell https://github.com/jasnell How about the following 2 static
methods which is defined at IDL:

• domainToASCII(domain)
• domainToUnicode(domain)

I didn't see those in the proposal :-(

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#28 (comment), or mute
the thread
https://github.com/notifications/unsubscribe/AAa2eVA3NG0j4lEZnCUSro0RREhVL5wBks5qHbXqgaJpZM4IrtaC
.

I'm not sure yet if theirs is a pure JS impl or not.

There is https://github.com/jsdom/whatwg-url

cc @domenic

Yeah, I'm familiar with (and use) whatwg-url. @domenic, how would you feel about the possibility of pulling the whatwg-url implementation into core? I'm not yet that familiar with how it is implemented internally but having a url standard compliant url parser built into core would be very good.

Yep, that's @Sebmaster's most excellent work. It's not super optimized, but would be a good starting point.

This thread is very exciting and I'm glad there is an appetite for the idea!! The idea of a global, the same as in browsers, is great.

I think there are two separable problems here:

• Parsing URLs according to the URL Standard. This could possibly even be done for require("url") in a semver-major bump, as mostly it will change edge cases.
• Introducing a URL Standard-compliant API for URL manipulation. There are a couple things to note here:
  • The URL Standard uses the USVString type, which (per a convoluted series of specs) essentially means that unpaired surrogates get censored. This is generally a good property, and recommended by Unicode, as network boundaries do not expect unpaired surrogates. But it's not immediately obvious from the algorithms.
  • The URL Standard takes great pains to ensure that the URL is always consistent and usable, by using setters and getters. So e.g. if you do url.pathname = "\uDC01", then that will initiate a parse in the pathname override state, and then if you trigger the getter, you'll find url.pathname === "/%EF%BF%BD". Note how this both appends the leading slash, and also performs the USVString surrogate pair censoring (decodeURIComponent("%EF%BF%BD").charCodeAt(0) === 0xFFFD, not 0xDC01). This is a fairly important property IMO.

There are tests of the URL Standard at https://github.com/w3c/web-platform-tests/tree/master/url, and whatwg-url has a runner. The coverage is pretty reasonable; see web-platform-tests/wpt#3018

@domenic, how would you feel about the possibility of pulling the whatwg-url implementation into core?

Oh, that'd be very cool! I guess my only concern is that we weren't concerned with speed when writing it, so there is probably lots of low-hanging fruit for performance improvements.

You'd also need to do a bit of work to decouple it from webidl-conversions and webidl2js. If you npm install it you'll find that it follows the same impl/wrapper strategy as browsers do, where there's a "wrapper" that takes care of USVString conversion and so on, delegating to the "impl" where parsing occurs, which in turn delegates to the URL state machine code. At least one of these layers could be disintermediated, although perhaps benchmarking should be done to see if that's actually the area where most improvement could be made.

Ok, I'll dig in and explore the whatwg-url internals and see what can be done reasonably. Before getting too deep into this I'd definitely like to get more +1's from collaborators tho. I'd really like to see this happen tho.

I think there are two separable problems here

Definitely agree that it's worth separating these. Not sure about modifying the existing require('url') too much tho -- ideally once the global URL is there for a while we could simply deprecate require('url') (and possibly even require('querystring')) entirely with an eye towards reducing the Node.js-specific API surface area.

Well, you can have a 👍 from me! More browser/server unity would be great. 😺

BTW, I like the idea of the global, since that's what browsers do, but I'd recommend not exposing it in require('url') because then people might get used to it being there rather than available globally, making future removal of the module more difficult.

The only concern I would have with that, @Qard, is that if there is existing code that does URL = whatever, there would be no way of recovering the original global. That should be a limited edge case, however.

This is not an argument for or against, but a request for more background. You've described what work you would like to do, and how you would do it — but I don't see much in the way of "why" we'd want to do this. What is not working about the status quo that we will rectify by adding a new global object? What value do Node users gain, concretely, from the URL object?

Maybe store it on process.URL for safe keeping?

@chrisdickinson personally I see quite a bit of benefit in minimizing the delta between node + the browser as far as utility API's like this are concerned.

@chrisdickinson ... the why is straightforward: Currently Node.js' URL parsing has a number of issues in terms of not following the standardized behavior implemented by browsers. Examples of those failures can be seen in the test case I referenced here. There are also differences in the Node.js provided API that are largely unnecessary. This work would give us an opportunity to not only provide more robust URL parsing, but to provide a unified, non-Node.js specific API.

@Qard ... that's certainly a possibility

FWIW, looks like I mis-remembered the number of failures ;-) ... here's the exact count:

bash-3.2$ ./node test/known_issues/test-url-parse-conformance.js 
Unknown globals: URL

assert.js:90
  throw new assert.AssertionError({
  ^
AssertionError: 160 failed tests (out of 352)
    at Object.<anonymous> (/Users/james/Node/main/node/test/known_issues/test-url-parse-conformance.js:57:8)
    at Module._compile (module.js:541:32)
    at Object.Module._extensions..js (module.js:550:10)
    at Module.load (module.js:458:32)
    at tryModuleLoad (module.js:417:12)
    at Function.Module._load (module.js:409:3)
    at Function.Module.runMain (module.js:575:10)
    at startup (node.js:152:18)
    at node.js:449:3
bash-3.2$ 

My general dislike of constructors aside, I have to second @chrisdickinson's concern here. It's not clear to me why we'd want this as a global. Sure, that's how browsers implement it, but Node is not a browser, and browser APIs have historically been designed with a fundamentally different model in mind (namely, the lack of CommonJS).

This proposal, in its current form, is starting to look like it'll drag Node in the direction of PHP - endlessly adding APIs and not really trying to enforce or encourage consistency. As it is, this will just further confuse users as to whether they should be require()ing a module or expecting a global to be there - not to mention it being unclear whether they should use the url module or the URL global. Bundling tools already take care of adding in 'fake' modules that point at browser APIs, so what exactly are we gaining here by trying to make it "more like browser APIs"?

As a separate question, how - if at all - does this cover the "insert object of URL components, receive stringified URL" usecase?

EDIT: Further question: Are there any major concerns about just fixing the existing url module in a next major (breaking) release? This would seem preferable to me, but I don't know if there are any major roadblocks that I might not be aware of, or whether this has been discussed somewhere before.

Let's separate the concerns just a bit. We could implement this but not make it a global, and just have it accessible via the existing url module (e.g. const URL = require('url').URL). We could go this route and still eventually deprecate the Node.js specific API. I would be ok with going that route. This proposes it as a global to be consistent with browsers, but that's not critical if we cannot get consensus on it.

Second, on the point:

endlessly adding APIs and not really trying to enforce or encourage consistency

My goal here is to eventually be able to fully deprecate the existing url and querystring modules with the hope of reducing the Node.js specific API surface area. Obviously that's not something that would happen quickly, tho, so the concern is definitely noted.

does this cover the "insert object of URL components, receive stringified URL" usecase?

It would in-so-far as the URL object as defined by the WHATWG spec includes toString() and the href property to provide serialization of the object. If what you're referring to is the ability to create a non-URL object and serializing it, that's not something that's currently supported by the WHATWG spec but it's something that can be easily maintained.

We could implement this but not make it a global, and just have it accessible via the existing url module (e.g. const URL = require('url').URL). We could go this route and still eventually deprecate the Node.js specific API. I would be ok with going that route. This proposes it as a global to be consistent with browsers, but that's not critical if we cannot get consensus on it.

That seems like a more workable solution to me. There would still likely be short-term user confusion while both APIs exist, however, so this would be something that'd require very clear documentation. I realize that you might've missed the question I added to my previous post later on - could you have a look at that suggestion as well?

My goal here is to eventually be able to fully deprecate the existing url and querystring modules with the hope of reducing the Node.js specific API surface area. Obviously that's not something that would happen quickly, tho, so the concern is definitely noted.

I understand that, but I still don't really see the value in trying to bring the Node.js API in line with browser APIs, as opposed to just keeping it internally consistent (given that Node.js was designed with CommonJS in mind, and switching to globals for everything would be a step back). In the end, Node.js is its own environment, and I can't see how trying to reduce the API to the lowest common denominator would benefit usability.

If what you're referring to is the ability to create a non-URL object a serializing it, that's not something that's currently supported by the WHATWG spec but it's something that can be easily maintained.

That's probably what I mean, yeah - I'm thinking of something along the lines of the url.format method, that can just directly accept a POJO.

EPS proposal updated. /cc @nodejs/ctc

Putting this on the ctc-agenda as I believe it's ready for discussion/review.

I would be against this. Browsers don't parse special schemes this way, and we should not either. git should be parsed the same as data, I believe.

Does npm rely on the parsing of urls with support for the git protocol? If so, do we have a migration path for that?

There's a certain loss of functionality by using new URL() over url.parse(). Specifically that url.parse() isn't just for URLs but also for URIs. For example: url.parse('github.com') works fine but new URL('github.com') throws.

Taking the following excerpt from the WHATWG URL specification:

Standardize on the term URL. URI and IRI are just confusing. In practice a single algorithm is used for both so keeping them distinct is not helping anyone. URL also easily wins the search result popularity contest.

It seems that URL is not meant to mean only URL. Even though there a scheme is always necessary (whether absolute or relative URL). Because of this any scheme is acceptable, but possibly parsed as expected. For example:

new URL('git://github.com/foo/bar').pathname == '//github.com/foo/bar';

Even though git: has been provisionally registered with the IANA it is still not parsed as a standard domain. It should be possible to properly properly parse the pathname from the above.

Taking from RFC3986 we can see a simple generic way of parsing the URL that I think could be used at minimum to parse unfamiliar schemes:

         foo://example.com:8042/over/there?name=ferret#nose
         \_/   \______________/\_________/ \_________/ \__/
          |           |            |            |        |
       scheme     authority       path        query   fragment
          |   _____________________|__
         / \ /                        \
         urn:example:animal:ferret:nose

@domenic

I would be against this. Browsers don't parse special schemes this way, and we should not either. git should be parsed the same as data, I believe.

Not to be rude, but check your own work first. Chrome has added an exception to URL() for its own needs:

new URL('chrome-extension://adlfkjaslkfj/foo/bar.html').pathname === '/foo/bar.html';

Additionally, URL() today doesn't parse unknown schemes as data:

const u = new URL('git://github.com/foo/bar');
u.protocol === 'git:';
u.origin === 'git://';

All this said, we should support other/unknown schemes properly with URL().

It's already been clarified upthread that I was incorrect about special schemes. There are no special schemes besides file:

@domenic oop. sorry about that. completely glanced over that comment.

Adding git as a special scheme would be trivial if necessary. If necessary
we should first try to upstream such a change into the URL spec itself.

On Jul 13, 2016 10:13 PM, "Trevor Norris" [email protected] wrote:

@domenic https://github.com/domenic oop. sorry about that. completely
glanced over that comment.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#28 (comment), or mute
the thread
https://github.com/notifications/unsubscribe/AAa2eczDLckC0l3fAaMSqdd27bHxSOJVks5qVcV-gaJpZM4IrtaC
.

After further testing, git URLs are handled by this implementation without any problems whatsoever using the implementation in #28.

The CTC discussed this today and has decided to advance this to DRAFT status with the exception of the URL constructor not being a global. I will get the PR updated to remove part about it being a global and will get it landed. Thank you all.

@yorkie ... eventually, once the new URL object is no longer experimental, my goal is to replace use of the existing url.parse within the http module entirely. That's not going to be easy tho. It's going to take a long time.

Landed in f16f770