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

Default value for process.env.NODE_ENV #35

Closed
davidturissini opened this issue Mar 26, 2019 · 1 comment · Fixed by #78
Closed

Default value for process.env.NODE_ENV #35

davidturissini opened this issue Mar 26, 2019 · 1 comment · Fixed by #78
Labels
kind/enhancement Categorizes issue or PR as related to a code enhancement.

Comments

@davidturissini
Copy link

Description

Building an app with observable-membrane as a dependency throws because process.env.NODE_ENV is not defined. In order to fix this, developer has to add rollup-plugin-replace or another tool that will strip out process.env.NODE_ENV. We should provide a default value so that installing this package doesn't immediately break builds.

Steps to Reproduce

  1. Start new app with rollup
  2. Install observable-membrane
  3. Build

Expected Results

No Error 🎉

Actual Results

Uncaught ReferenceError: process is not defined 😿

@pmdartus
Copy link
Member

IMO process.env.NODE_ENV should stay in the generated esm modules. Providing a default value means that the bundle can't do dead-code elimination.

Popular libraries like React, Redux or Vue or ship process.env.NODE_ENV in their generated lib and rely on bundler to inject the right values. While rollup requires a plugin, it's done by default by Webpack and browserify. The only format where it makes sense to strip it is for UMD.

@pmdartus pmdartus added the kind/enhancement Categorizes issue or PR as related to a code enhancement. label May 25, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/enhancement Categorizes issue or PR as related to a code enhancement.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants