Defining global: 'globalThis' variable will replace global.css as "globalThis".css on building

[JounQin]

Describe the bug

As title

Reproduction

https://github.com/JounQin/test/tree/vite_define

System Info

System:
    OS: macOS 12.0
    CPU: (16) x64 Intel(R) Core(TM) i9-9880H CPU @ 2.30GHz
    Memory: 17.84 GB / 64.00 GB
    Shell: 5.8 - /bin/zsh
  Binaries:
    Node: 12.22.5 - ~/.nvm/versions/node/v12.22.5/bin/node
    Yarn: 1.22.11 - /usr/local/bin/yarn
    npm: 6.14.14 - ~/.nvm/versions/node/v12.22.5/bin/npm
  Browsers:
    Chrome: 92.0.4515.159
    Firefox: 91.0.2
    Safari: 15.0
  npmPackages:
    vite: ^1.0.0-rc.13 => 1.0.0-rc.13

Used Package Manager

yarn

Logs

vite:config env mode: production +0ms
  vite:config env: {} +1ms
  vite:config config resolved in 94ms +1ms
Building production bundle...
  vite:build:resolve /src/main.js --> /Users/JounQin/Workspaces/Local/test/src/main.js +0ms
[vite] Build errored out.
Error: Could not resolve './"globalThis".css' from src/main.js
    at error (/Users/JounQin/Workspaces/Local/test/node_modules/rollup/dist/shared/rollup.js:151:30)
    at ModuleLoader.handleResolveId (/Users/JounQin/Workspaces/Local/test/node_modules/rollup/dist/shared/rollup.js:19862:24)
    at /Users/JounQin/Workspaces/Local/test/node_modules/rollup/dist/shared/rollup.js:19856:26 {
  code: 'UNRESOLVED_IMPORT',
  watchFiles: [
    '/Users/JounQin/Workspaces/Local/test/index.html',
    '/Users/JounQin/Workspaces/Local/test/src/main.js',
    '/Users/JounQin/Workspaces/Local/test/node_modules/@vue/runtime-dom/dist/runtime-dom.esm-bundler.js',
    '/Users/JounQin/Workspaces/Local/test/node_modules/@vue/runtime-core/dist/runtime-core.esm-bundler.js',
    '/Users/JounQin/Workspaces/Local/test/node_modules/@vue/shared/dist/shared.esm-bundler.js',
    '/Users/JounQin/Workspaces/Local/test/node_modules/@vue/reactivity/dist/reactivity.esm-bundler.js',
    '/Users/JounQin/Workspaces/Local/test/src/App.vue',
    '/Users/JounQin/Workspaces/Local/test/src/App.vue?vue&type=script&lang.js',
    '/Users/JounQin/Workspaces/Local/test/src/App.vue?vue&type=template&id=601a3950&lang.js'
  ]
}

Validations

• Follow our Code of Conduct
• Read the Contributing Guidelines.
• Read the docs.
• Check that there isn't already an issue that reports the same bug to avoid creating a duplicate.
• Make sure this is a Vite issue and not a framework-specific issue. For example, if it's a Vue SFC related bug, it should likely be reported to https://github.com/vuejs/vue-next instead.
• Check that this is a concrete bug. For Q&A open a GitHub Discussion or join our Discord Chat Server.
• The provided reproduction is a minimal reproducible example of the bug.

[JounQin]

Oh, it's "globalThis".css after transformed, it is really bad to auto JSON.stringify variables.

[y1d7ng]

try GLOBAL instead of global in define, described in the doc

Because it's implemented as straightforward text replacements without any syntax analysis, we recommend using define for CONSTANTS only.

[JounQin]

@OneNail I'm trying to define global for 3rd-party modules like available-typed-arrays.

It is a very common usage for a bundler.

[haoqunjiang]

No, it's not common. Have you ever seen webpack recommend users to use DefinePlugin for things other than constants?

Should use globalThis.global = globalThis instead.

[JounQin]

@sodatea

No, it's not common. Have you ever seen webpack recommend users to use DefinePlugin for things other than constants?

Should use globalThis.global = globalThis instead.

It's a bad practice to modify globalThis AFAIK, even worse for a variable I'm not using.

Have you ever seen webpack recommend users to use DefinePlugin for things other than constants?

Using global: 'globalThis' in DefinePlugin is fine, it works perfectly.

webpack@<=4 polyfills global automatically, webpack@5 requires users to define it manually.

[JounQin]

See also inspect-js/has-symbols#11 (comment) and browserify/node-util#62 (comment)

I'm raising PRs for many related packages to use globalThis if available, like inspect-js/available-typed-arrays#13, inspect-js/which-typed-array#52 and inspect-js/is-typed-array#53

But, define should be more powerful IMO.

[haoqunjiang]

It's a bad practice to modify globalThis AFAIK, even worse for a variable I'm not using.

Polyfills do this all the time.

Using global: 'globalThis' in DefinePlugin is fine, it works perfectly.

"is fine" != "recommended". Webpack implemented a dedicated plugin for the global shimming, instead of reusing DefinePlugin https://webpack.js.org/configuration/node/#nodeglobal

But, define should be more powerful IMO.

Yeah, but not for this use case.

[JounQin]

Polyfills do this all the time.

webpack@4 polyfills global by transforming, not globalThis.global = globalThis.

Webpack implemented a dedicated plugin for the global shimming, instead of reusing DefinePlugin

Then, this feature is missing in vite.

[haoqunjiang]

Then, this feature is missing in vite.

Feel free to write a plugin for this. Should be very straightforward.

Or you can check out https://github.com/snowpackjs/rollup-plugin-polyfill-node I'm not sure if it works though.

[JounQin]

Feel free to write a plugin for this. Should be very straightforward.

Yeah, I'm going to write a plugin maybe vite-plugin-provide.

Or you can check out snowpackjs/rollup-plugin-polyfill-node I'm not sure if it works though.

This one won't work on vite development, at least.

[ljharb]

A node module bundler that does not provide node globals is broken; it's unfortunate vite won't try to function correctly.

[haoqunjiang]

A node module bundler that does not provide node globals is broken

I thought there was a consensus that bundlers should not try to shim Node.js built-ins. It's bad for the ecosystem.

Why rely on a runtime-specific global variable instead of the language built-in?

[ljharb]

There is no such consensus. Webpack 5 made this disruptive change, and package maintainers find it very hostile and disruptive: https://blog.sindresorhus.com/webpack-5-headache-b6ac24973bf1

CJS modules, which are the majority of npm and will remain so for the foreseeable future, are already node-specific - if you take them as input, you have already decided to treat node as special, and CJS modules can not function properly unless they run in a node-like environment, which includes all the builtins.

[haoqunjiang]

From the exact article you linked:

Honestly, Webpack not polyfilling does make sense in theory. I just think they did it too early and with too little consideration about its effects on the ecosystem. I imagine this change would have been much easier to do in some years when a lot more Node.js package are ESM-only and Node.js supports more browser APIs.

The timing is different now. With Sindre Sorhus himself aggressively publishing ESM-only packages, and with the ESM-centric workflow that Vite is pushing forward, the decision should make sense.

---

On the other hand, CJS, as a runtime-agnostic module specicifcatiton, exists before Node.js.
It is as widely used for client-side JavaScript modules as Node.js modules, if not more. These CJS modules can not function properly unless they run in a browser-like environment.

Vite supports the CJS module system, not CJS Node modules. It doesn't treat Node as special.
You can access Node.js built-ins when serving for SSR, and DOM APIs when you are bundling for browsers.

[ljharb]

The timing isn't actually different now; when a package goes ESM-only, what happens is that everyone stops using it in favor of a less hostile package.

Either way, an ESM node module still expects node globals and core modules, so that has no relevance to the issue at hand.

CommonJS existed before node.js, but that version of it is long dead, far more obsolete than even AMD. The only CJS that matters now (and for the better part of a decade) is node's module format.

Basically none of the packages on npm are CommonJS modules - virtually all of them are node CJS modules. Node is special, and will always be, and you only hurt users when you pretend it's not.

[haoqunjiang]

I respectfully disagree.

Those packages runs in browsers but still expects Node.js built-ins are more like "webpack (<= 4) compatible CJS modules" than Node.js CJS modules.

Please note that the webpack's polyfills were neither complete (some were empty, some were impossible to implement on the browser side), nor up-to-date (some dependencies of https://github.com/webpack/node-libs-browser were several versions behind the latest), sometimes ambiguous or even incorrect (the result of __filename and __dirname in browser environment would surprise lots of people).

Adding these polyfills into the browser bundle by default not only bloats the bundle, but also introduces potential bugs.

This hurts the ecosystem more than leaving it to the users.

[JounQin]

I think another point is that vite does not provide a polyfill functionality like webpack@5 as you linked https://webpack.js.org/configuration/node/#nodeglobal

[haoqunjiang]

I think another point is that vite does not provide a polyfill functionality like webpack@5 as you linked webpack.js.org/configuration/node/#nodeglobal

For the remaining shims in webpack 5:

• The global shim is a one-liner.
• __filename and __dirname are ambiguous. They should not be shimmed in the first place. If you do need it, you should know the consequences and write your own shim (the default mock behavior is a one-liner, too)

Even if you don't feel like polluting the global namespace, a custom plugin won't take much more effort. (I would usually avoid global variables, too. But in the global case, I think it already suffices and is harmless)

[ljharb]

They’re not ambiguous whatsoever - they refer to the file they’re in. The filesystem is an inextricable part of file-based modules.

It’s not a global namespace - node modules run in a smaller scope than that, and require is precisely the same as __dirname.

[haoqunjiang]

They’re not ambiguous whatsoever

__filename is ambiguous in the context of a front-end bundler.

What should it be relative to? The input file? The output file? Or should it be left untouched because it might be executed in an SSR context too?
And maybe to your surprise, the default behavior in webpack 4 is to use a fixed value "index.js".
So if your module works well in webpack 4 by default, it may still break if another bundler tries to set any other default.

and require is precisely the same as __dirname

CJS spec is well documented, well tested, and widely used. Vite can support it, even if imperfectly. Because the edge cases are predictable and understandable.

But Node.js APIs in browser? Not so much.
Package authors shouldn't expect it to be well supported in the first place.

[ljharb]

Literally nobody uses the CommonJS spec - everyone is using "node's CJS modules", again for the better part of a decade.

node APIs in a browser have been used by browserify and thus webpack (prior to v5, and perhaps v4) for also the better part of a decade, and a great many packages do rely on it. It's a reasonable expectation because that's how it's always been.

Perhaps this wasn't a good choice initially! That doesn't change that it is the way it is, and what webpack 5 and friends are doing right now is trying to fight "what is" in a disruptive and hostile way in an effort to force an ecosystem they prefer. Please don't join in that disruptive effort.

[yyx990803]

@ljharb the mainstream tooling authors have reached the consensus that Node is no longer special - with Deno and worker
environments becoming more common, it's just one of the many JS runtimes that build tools may target. Its platform-specific APIs should not get special treatment in general-purpose build tools, especially when the user is targeting browsers. JS packages published on npm are also no longer by definition Node-specific - if a package is meant to be usable in the browser, then it is the package authors' responsibility to ensure their code can run properly in the browser without special treatment.

You may disagree with that and that's fine - but your opinion really doesn't matter here, because the ecosystem moves forward as a result of the natural selection process of its users. If not supporting Node built-ins is a mistake, surely fewer users would use tools that don't support them (including Vite and webpack 5), and those tools should die out in the long run. If users turn out to still opt for these tools, then it means to them the utility these tools provide are more important than the occasional minor inconvenience, and in turn they would request the package authors to properly ship universal packages (instead of relying on Node APIs as if it is available in all JS environments). That's how we shed the collective technical debt and move the ecosystem forward, and it's 100% an intentional decision in Vite's design to push it towards that direction.