Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reliance on process.env.NODE_DEBUG #77

Closed
ghost opened this issue Aug 1, 2022 · 6 comments
Closed

Reliance on process.env.NODE_DEBUG #77

ghost opened this issue Aug 1, 2022 · 6 comments

Comments

@ghost
Copy link

ghost commented Aug 1, 2022

This code will fail if process or process.env does not exist:

node-util/util.js

Lines 109 to 110 in 6b82825

if (process.env.NODE_DEBUG) {
var debugEnv = process.env.NODE_DEBUG;

We ran into this with vite.js 2.8.x which has another convention (https://vitejs.dev/guide/env-and-mode.html#env-variables).
node-util should probably have a safe fallback for when this environment variable does not exist (assume production build?).

We used a workaround described here: visgl/deck.gl#6134 (comment)

@ljharb
Copy link
Member

ljharb commented Aug 2, 2022

This is a node module; they always exist.

If vite doesn’t provide then, it’s not a node module bundler.

@ljharb ljharb closed this as not planned Won't fix, can't repro, duplicate, stale Aug 2, 2022
@ghost
Copy link
Author

ghost commented Aug 16, 2022

For people who see this in the future, there's also #43 which discusses this in more detail, and provides solutions for different tools which aren't node module bundlers.

@eddiemonge
Copy link

eddiemonge commented Nov 14, 2022

This is a node module; they always exist.

That's not a good reason for not including a simple check like

if (process && process.env && process.env.NODE_DEBUG) {

Just because this may be a node module doesn't mean it will always run in node. Node alternatives are popping up that may want to use this to get node like things but may not expose process.

The README also states

This implements the Node.js util module for environments that do not have it, like browsers.
which implies this is definitely not a node module.

edit
After reading #43, the issue is far deeper, but this one causes immediate breaking.

@ljharb
Copy link
Member

ljharb commented Nov 15, 2022

Yes, it does mean that. An X module only ever runs in X, or in X-like environments.

@manzoorwanijk
Copy link

This is a node module; they always exist.

@ljharb If it always exists, then may be we should remove these checks for typeof process?

node-util/util.js

Lines 76 to 82 in ef98472

if (typeof process !== 'undefined' && process.noDeprecation === true) {
return fn;
}
// Allow for deprecating things in the process of starting up.
if (typeof process === 'undefined') {
return function() {

I don't understand the rigid behavior of denying to add a simple safety check. 🤷‍♂️

#62 should have been merged a long time ago. At least, it doesn't hurt or break anything.

@ljharb
Copy link
Member

ljharb commented Mar 17, 2024

@manzoorwanijk true, we should remove those.

What it harms is that it furthers the tolerance of broken bundlers in the ecosystem.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants