url is defined on globalThis. url is far too common a variable name

guest271314 commented 2 months ago

What version of Bun is running?

1.1.20

What platform is your computer?

x64

What steps can reproduce the bug?

url is defined on globalThis. To access URL we have to use url.URL.

url is far too common a variable name used in development and production to be defined globally. URL should be global. Change the global variable url to something else.

What is the expected behavior?

url to not be defined on globalThis.

What do you see instead?

url is already defined on globalThis.

Additional information

No response

A-D-E-A commented 2 months ago

I see that it is also defined globally in Node (but not in browsers nor Deno). So I think it should be kept for compatibility issues. It might break some package somewhere.

But this behavior should be documented. It is neither documented in bun nor node (from what I could find on each website's doc section about globals).

That said, I found there are a few differences between Bun and Node regarding this special variable.

Listing the props in Bun.

Listing in Node.

Trying to reassign in both.

guest271314 commented 2 months ago

I see that it is also defined globally in Node (but not in browsers nor Deno).

URL is defined globally in browsers and in deno, and node https://developer.mozilla.org/en-US/docs/Web/API/URL#browser_compatibility.

Strangely node has duplicate entries for URL, it's global and defined on url.

Just because something is done in Node.js world doesn't mean it's a good idea. CommonJS is not the specified module loader for JavaScript, Ecmascript Modules are.

A-D-E-A commented 2 months ago

URL is defined globally in browsers and in deno, and node https://developer.mozilla.org/en-US/docs/Web/API/URL#browser_compatibility.

Strangely node has duplicate entries for URL, it's global and defined on url.

The global url is not the same as URL (or url.URL), which is also not the same as url.Url. Different interfaces implement different methods. Not all of them have parse or canParse which is why we have multiple interfaces. For example, in Node, the method url.resolve is present only in url and not accessible in URL.

Just because something is done in Node.js world doesn't mean it's a good idea.

While I do agree completely that it is not a good idea to have this global variable, the main selling point of Bun is that it is almost a drop-in replacement.

With a quick grep, I could find many instances of url.resolve in the dependencies of one of my projects. Here's a quick list of some npm libraries that do use this method (and not just in type declarations) and some of their dependents which could all break if we remove url.resolve:

@mapbox/node-pre-gyp
- bcrypt
- grpc
- libxmljs
- @tensorflow/tfjs-node
- node-libcurl
- odbc
- duckdb
- canvas
  - ascii-art
  - chartjs-node-canvas
  - discord-canvas
  - chartjs-node
- node-gtk
@vitest/coverage-c8
vite
node-gyp
- node-sass
- nodegit
- re2
- electron-rebuild
- duckdb
registry-auth-token
- package-json

And that's just a few of the most popular/depended upon packages. The total is well in the thousands.

I can't speak for the maintainers of these packages or for the maintainers of Bun, but I personally feel that breaking all of these dependencies is not worth it.

guest271314 commented 2 months ago

For example, in Node,

Bun ain't Node.js. Not even close.

While I do agree completely that it is not a good idea to have this global variable, the main selling point of Bun is that it is almost a drop-in replacement.

Again, not even close. I think that is a poor selling point, if something is trying to sold. Bun is Bun.

If Bun was Node.js we wouldn't still be waiting for HTTP/2. I run Bun because it ain't Node.js, it's Bun.

I'm talking about WHATWG URL https://url.spec.whatwg.org/#url. Not some Node.js-specific API that Bun arbitrarily decides to copy in to Bun.

Jarred-Sumner commented 2 months ago

url is defined in the repl, not in scripts. This behavior is consistent with Node.

guest271314 commented 2 months ago

@Jarred-Sumner Sure enough URL is defined globally.

bun run test.js

console.log(URL);

for (const prop in URL) console.log(prop);

[class Function]
parse
canParse
createObjectURL
revokeObjectURL

This behavior is consistent with Node.

I think that ship (and claim) has sailed.

Bun is it's own thing. I would strongly suggest to consider abandoning that claim, and just focus on doing Bun, for Bun's sake. I started using Bun, not because Bun has Node.js' behaviour, but because Bun is different from Node.js, which nobody can really say it's not.

oven-sh / bun