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

Added process npm package dependency to fix the ReferenceError #55

Closed
wants to merge 1 commit into from
Closed

Added process npm package dependency to fix the ReferenceError #55

wants to merge 1 commit into from

Conversation

himanshuapril1
Copy link

@himanshuapril1 himanshuapril1 commented Dec 23, 2020

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

  • Fixes ReferenceError: process is not defined in browser and while bundling with Webpack 5.

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

@ljharb
Copy link
Member

ljharb commented Dec 23, 2020

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
Copy link

@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
Copy link
Member

ljharb commented Jan 17, 2021

@mkoubik
Copy link

mkoubik commented Feb 12, 2021

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

@ljharb
Copy link
Member

ljharb commented Feb 12, 2021

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
Copy link

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

@veryeasily
Copy link

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
Copy link
Member

ljharb commented Apr 3, 2021

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.

This pull request was closed.
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

Successfully merging this pull request may close these issues.

Undeclared use of node's process causes exception in browser
6 participants