node-util
node-util copied to clipboard
browserify
Don't assume process.env exists in the global scope.
Some platforms may not have process.env in the global scope and this shouldn't prevent util from running. Currently it will thrown an exception on import if process.env doesn't exist.
This is a node module; process.env always exists.
A bundler bundling this node module is broken if it doesn't ensure this code works in the target environment. Are you using such a broken bundler?
This is a node module; process.env always exists.
Why do other parts of the file do this "typeof process !== 'undefined'" check then? e.g. https://github.com/browserify/node-util/blob/master/util.js#L76 . It seems inconsistent to me.
Are you using such a broken bundler?
No, I'm using node-util an a platform that doesn't have process in the global scope. Other than this minor problem all the other code works fine.
Which platform?
Hermes https://hermesengine.dev/ . But does it matter? From the description of node-util, it is designed to work in any environment right?
Node.js's util module for all engines. This implements the Node.js util module for environments that do not have it, like browsers.
Yes, it does matter. node modules can only run unmodified on node - they often use require, for example - and a build tool is always required for other platforms, whether server or browser.
That the file already has a typeof process check does seem to suggest that this change is reasonable, though.
Any news? This package is used by assert package, which is a built-in node module, but some packages can also be run on browser like micromark use assert inside, so I have to install npm assert on development and strip them on production building.
For now, I have to define a process.env.NODE_DEBUG in vite.config.ts for workaround.
File a bug on vite, if it’s broken the same way webpack 5 is. A node module bundler must be able to transparently provide browser versions of node core module and globals, including process.
File a bug on vite, if it’s broken the same way webpack 5 is. A node module bundler must be able to transparently provide browser versions of node core module and globals, including process.
@ljharb
There is workaround by custom defined process.env. NODE_DEBUG as I described.
I don't think it is a bug of vite, not of node-util neither of course.
But like https://github.com/inspect-js/has-symbols/issues/11#issuecomment-787098519, adding a check here is great.
It's definitely a bug, because it's talking node modules as input and not providing node globals.
In the case of has-symbols, it works identically because it's a language global, available in both node and browsers. In this case, when process is undefined, the precondition for running this entire package has failed to be met, and the entire environment is invalid.
@ljharb
Personally, I don't think polyfilling process or any other node specific features is the design philosophy of vite, just like webpack@5, the end users must polyfill them manually.
@JounQin i'm aware of that, but it being their design philosophy doesn't prevent it from being wrong.
@JounQin i'm aware of that, but it being their design philosophy doesn't prevent it from being wrong.
@ljharb That's why we're here talking about how to support them.
It is said in README:
This implements the Node.js util module for environments that do not have it, like browsers.
If your code runs in the browser, bundlers like browserify or webpack also include the util module.
The best way to support them is to advise in the readme how users can fix these broken bundlers' mistakes, and to link them to the relevant issues to help surface the pain it's causing.
That the file already has a typeof process check does seem to suggest that this change is reasonable, though.
The other typeof process check is kind of vestigial, it (used to?) exist in Node.js in case the module runs during startup when process is not yet available, but when this was last copy-pasted other parts of the Node.js code already relied on process being available from the start.
Its tempting to just add the check since I don't think anyone has ever used NODE_DEBUG in the browser, but that would make it a breaking change if an additive feature (in node.js) relies on the process global in the future. So i do agree with ljharb that the correct approach here is to document what users need to do.
IIRC it's something like this:
npm install util process
module.exports = {
plugins: [new ProvidePlugin({ process: 'process/browser' })],
resolve: {
fallback: {
process: require.resolve('process/browser'),
util: require.resolve('util/')
}
}
}
If there is a webpack plugin that just provides ALL the Node.js builtins that would be ideal…
I ran across this issue in the following way: Our application uses sinon for browser tests. Sinon has a require('util') call but does not actually have util as a dependency and this attempt would fail silently. However, we also installed aws-sdk in the same monorepo for a command line tool. This does have util as a dependency. Now that util was in our tree, sinon started being able to require it. At this point we started seeing failures because process was not defined. I have no interest in adding process to our dependencies since we have no other reason for it. I would be happy to see the process check extended further.
@wagenet that's because util is a core module and should NOT be a dependency (same with process). This package is silently substituted in by bundlers (ones that work properly, at least).
In my opinion, this discussion is pointless and the PR should be merged. A checker to get if process.env exists will only help to make the code more compatible with a wide range of projects. I got problems by myself with this topic, and had to clone the code myself and make changes to ensure that I was defining process.env before the usage. Looks like we are having a dystopic ego question in a simpler question that appeared almost 6 months ago.
I am not getting it. What is the potential harm in adding a safe check?
@parikshit223933 enabling broken tools to work anyways is harmful for the ecosystem; this also adds extra bytes for every user, despite only users of broken tools needing it.
Your point is 100% valid. Should we remove typeof process !== 'undefined' on line 76 as well?
This is a node module; process.env always exists.
This is not a Node module, but rather it's a polyfill of a Node module for the browser. Browser code would really be better distributed as ESM instead of CJS. Then it could be used directly without bundling. At that point it'd make a lot of sense not to assume that process.env exists as that's something that exists only in Node and not in the browser
@benmccann that's its purpose, to be used in a browser, but it has package.json - it's a node module. It's meant to work with bundlers, the standard for which is browserify, which invented the concept. A bundler that deviates from this defacto standard is broken, and it would be harmful to make changes that enable people to use broken bundlers.
As for "without building", that's just not feasible in modern web dev, and native ESM is both slower and less fully-featured than CJS, and, since ESM can't be consumed by CJS, but CJS can be consumed by ESM, CJS is the more universal format (assuming a build process exists).
It's a bit rich to me that you would pick the least used of all bundlers and call it the standard. Just because something's first, doesn't mean that's what the standard is. Today Chrome, Firefox, and Safari set the standards for web browsers. No one goes around screaming that they're breaking standards because they're deviating from the behavior of Nexus and Mosaic. If my memory serves me correctly esbuild, Vite, and Rollup do not polyfill process.env while webpack and browserify do. So I believe the majority of users are using bundlers which do not polyfill process.env. And similarly, while CJS was first, the standard is ESM.
https://npmcharts.com/compare/browserify,vite,webpack,rollup,esbuild
Webpack 5 no longer does, I believe, which is why it's a huge problem for both users and maintainers (https://blog.sindresorhus.com/webpack-5-headache-b6ac24973bf1 unfortunately no longer loads), and why, I suspect, webpack 4 on npm has 7m downloads a week and webpack 5 has merely 1.6m.
esbuild, vite, and rollup making this choice also causes no end of filed issues on projects, and the solution for all of them is "sadly, your bundler forces you to configure it properly instead of doing it by default".
CJS is a standard, ESM is not "the" standard - it's just one of multiple.
webpack 5 actually has more usage than webpack 4. It's just that webpack 5's usage is split amongst several versions while webpack 4's usage is concentrated in the terminal version. E.g. these half a dozen version have over 9m downloads between them:
5.83.1 2,135,676
5.82.1 1,186,589
5.76.1 1,152,487
5.75.0 2,361,856
5.74.0 1,655,382
5.73.0 689,447
At this point, I'm not really sure what the argument is for continuing to use process.env. You can argue that bundlers should polyfill it, but the fact is that none of the major bundlers do that and they're moving in the opposite direction. At this point, continuing to use process.env is not practical and would just be causing everyone pain to fight a losing ideological battle.
Thanks, fair point on webpack 5 usage - it's still barely more than half, after being out for 3 years.
This is a node module. process.env always exists. If none of the major bundlers are doing the right thing, then they should be persuaded to do so - pressuring individual maintainers to cave, one package at a time, isn't an ethical or sustainable or practical approach.
For folks using Yarn, here's a gist to patch the package if you want this functionality.
https://gist.github.com/arinthros/68e1b622b27b98f3a6a45585dc751b82
Apparently there is someone marking useful comments as abuse. I suppose that who uses this repository basically changes the lib to use another version of the polyfill since the contributors can't stand being wrong.
@scarletquasar it is not useful to suggest people patch software they don't maintain, it is harmful.