browserify / node-util

node.js util module as a module
MIT License
247 stars 88 forks source link

Added process npm package dependency to fix the ReferenceError #55

Closed himanshuapril1 closed 2 years ago

himanshuapril1 commented 3 years ago

Since we're using process package without having it as a core dependency couple of things starts falling with latest adoption of Webpack 5 and other bundling tools. So in order to fix that dependency issue adding process package dependency.

Fixes #43

What does it fixes

cc @mischnic @devongovett @padmaia @stacylondon @goto-bus-stop

ljharb commented 3 years ago

The proper solution is for webpack 5 to fix its defaults to shim node core modules; failing that, webpack 5 users should be altering their config to fix it for themselves.

All that said, if this PR actually fixes the issue, then that seems like a reasonable compromise in the face of webpack 5's unfortunate decision.

(i also edited your OP; your original "resolves issue" format will fail to link the issue to the PR, and to autoclose the issue when the PR lands)

CrackersAreNotCookies commented 3 years ago

@ljharb You mentioned that webpack 5 users should be altering their config to fix it for themselves, but can you actually showcase how? I've been trying to import callbackify, but it keeps throwing the ReferenceError and I'm not sure how to solve it without an update on the util lib itself.

ljharb commented 3 years ago

https://github.com/nodejs/readable-stream/pull/455/files has an example.

mkoubik commented 3 years ago

It's also broken in vite.js 2.0 would't simple if (typeof process !== undefined) be enough?

ljharb commented 3 years ago

no, because in a working node module bundler, that condition would always be true.

not sure what vite is - is it using something besides webpack 5?

Dgiulian commented 3 years ago

Having the same issues with Vite.js 2.0. I believe this PR fixes the issues without assuming every user uses webpack 5.

veryeasily commented 3 years ago

Yeah, vite is an alternative to webpack. I was running into this issue with vite also, but managed to get around it with:

  define: {
    "process.env.NODE_DEBUG": "''",
  },

in vite.config.js.

This PR seems like a good solution to me since it assumes as little about the environment as possible. It might be a good idea to make process be a peer dependency instead of a normal npm dependency. That way environments that already have a global process don't need to pull down an extra module.

ljharb commented 3 years ago

A node module should assume it's running in node. A node module bundler should meet those assumptions. A node module intended to be bundled should assume both of those things as well.

"assumes as little about the environment as possible" isn't really a goal for node modules.

Peer dependencies are always required; everyone always has to pull it down, whether they need it or not. There is no "conditional dependency" concept in npm beyond "optional dependencies", which are for things that need to be compiled on install, which doesn't apply here.