lib: use non-symbols in isURLInstance check

[codebytere]

This slightly changes the conditional used to determine whether or not something is a URL instance. Since Node.js adds symbols to the URL not specified by the WHATWG, those symbols are not present in other implementations (like Blink's) and therefore can cause false negatives. In Electron's renderer process, running e.g. this code:

const fs = require('fs');
const path = require('path');

const url = new URL('file://' + path.resolve('index.html'));

const stream = fs.createReadStream(url);

stream.pipe(process.stdout);

would cause the following error:

"Uncaught TypeError: The "path" argument must be of type string or an instance of Buffer or URL. Received an instance of URL", source: internal/fs/utils.js (541)

To remedy this, I think we should instead switch on properties guaranteed to be present in all URL instances as specified by the WHATWG.

cc @jasnell

Checklist

• make -j4 test (UNIX), or vcbuild test (Windows) passes
• commit message follows commit guidelines

[nodejs-github-bot]

Lite-CI: https://ci.nodejs.org/job/node-test-pull-request-lite-pipeline/4203

[jasnell]

Let's use this as part of the argument back to TC-39 that URL really ought to be added to the language ;-) ... LGTM!

[devsnek]

Does this mean that in electron there are two different implementations of URL floating around?

[jasnell]

Sadly, yes, but I believe the blink one is the one that's actually global. The Node.js one would still be accessible via require('url'). As I alude, what I'd really like to see is URL added to JavaScript directly as an intrinsic -- yes with a normative reference to the WHATWG spec -- and provided to environments by v8.

[codebytere]

@devsnek heh well "floating around" is a bit nebulous, but two can exist depending on where/how you're invoking new URL. In the main process, it will always be the Node.js implementation, and in the renderer process without nodeIntegration: true enabled in webPrefs it will be Blink by default. When nodeIntegration: true is set webPrefs, it will still be Blink's by default, unless you invoke const { URL } = require('url') in which case it will then be the impl provided by Node.js.

Renderer example:

const fs = require('fs');
const path = require('path');

const url1 = new URL('file://' + path.resolve('index.html'));

const url = require('url')
const url2 = new url.URL('file://' + path.resolve('index.html'));

console.log(url1) // Blink impl
console.log(url2) // Node.js impl

will show:

[nodejs-github-bot]

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

[nodejs-github-bot]

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

[Trott]

Landed in 8825eb4.