Odd behaviour with env inlining and dead-code elimination

[benoitgrelard]

🐛 bug report

We're using library targets at Radix UI and need to build un-minified whilst also preserving our process.env.NODE_ENV dev checks so our users can build their apps with/without dev warnings.

From what we understand:

• --no-optimize isn't needed for libraries because of this:

  Enables or disables optimization (e.g. minification). Exact behavior is determined by plugins. By default, optimization is enabled during production builds (parcel build), except for library targets.

  source: https://parceljs.org/features/targets/#optimize

• NODE_ENV is set to production by default in parcel build, but can be overriden in shell/package.json so this is what we do in the package.json below:

  The NODE_ENV environment variable is automatically set by Parcel depending on the mode. When running parcel build, NODE_ENV is set to production by default, otherwise it is set to development. This can be overridden by setting NODE_ENV yourself (e.g. in your shell).

  source: https://parceljs.org/features/node-emulation/#node_env

• we can prevent inlining env setting "inlineEnvironment": false in package.json:

  Inlining of environment variables and readFileSync calls can be disabled by creating a @parcel/transformer-js key in package.json. This is what we do in the package.json below:

  source: https://parceljs.org/features/node-emulation/#disabling-these-features

🎛 Configuration (.babelrc, package.json, cli command)

Package.json

{
  "name": "my-library",
  "version": "1.0.0",
  "source": "src/index.js",
  "main": "dist/main.js",
  "module": "dist/module.js",
  "devDependencies": {
    "parcel": "^2.5.0"
  },
  "scripts": {
    "watch": "parcel watch",
    "build": "NODE_ENV=development parcel build"
  },
  "@parcel/transformer-js": {
    "inlineEnvironment": false
  }
}

.env

NAME=benoit

src/index.js

export function add(a, b) {
  console.log("process.env.NAME", process.env.NAME);
  console.log("process.env.NODE_ENV", process.env.NODE_ENV);

  if (process.env.NAME === "benoit") {
    console.log("name is benoit");
  }

  if (process.env.NODE_ENV === "development") {
    console.log("env is development");
  }

  return a + b;
}

🤔 Expected Behavior

This is the output we would expect:

function $cf838c15c8b009ba$export$e16d8520af44a096(a, b) {
    console.log("process.env.NAME", "benoit");
    console.log("process.env.NODE_ENV", "development");
    if (process.env.NAME === "benoit") {
        console.log("name is benoit");
    }
    if (process.env.NODE_ENV === "development") {
        console.log("env is development");
    }
    return a + b;
}


export {$cf838c15c8b009ba$export$e16d8520af44a096 as add};
//# sourceMappingURL=module.js.map

😯 Current Behavior

This is the output we are seeing:

function $cf838c15c8b009ba$export$e16d8520af44a096(a, b) {
    console.log("process.env.NAME", undefined);
    console.log("process.env.NODE_ENV", "development");
    console.log("env is development");
    return a + b;
}


export {$cf838c15c8b009ba$export$e16d8520af44a096 as add};
//# sourceMappingURL=module.js.map

Note 2 different issues are present:

• process.env.NAME wasn't inlined, but instead was completely lost/not loaded (undefined) meaning that we are even completely missing our log in the output
• the if statements are completely missing so we cannot deliver our users with dev checks

🔦 Context

As explained in the description, we are trying to achieve dev builds of our component library. The build output needs to be un-minified and retain all the process.env.NODE_ENV checks so that users can build their apps in dev or production mode with/without the dev warnings.

Reading #4993, it seems NODE_ENV is specifically singled out to be always inline regardless of "inlineEnvironment": false.

• What is the reason for that?
• How can one achieve a library output which retains dev checks for users?

It seems regardless of inlining or not, something else is wiping the checks anyway. I was under the impression that dead code elimination was performed by the optimizer (terser) which in this case isn't supposed to be running as these are library targets…

💻 Code Sample

Provided above ☝️

🌍 Your Environment

Software	Version(s)
Parcel	^2.5.0
Node	16.14.2
npm/Yarn
Operating System	MacOS

[devongovett]

What you described makes sense. For library targets, I think we should leave process.env expressions as is for environment variables that aren't defined, rather than replacing with undefined. I think we should still replace defined variables by default because libraries might use them to implement feature flags for example.

NODE_ENV is a bit special because many libraries use it for dev/prod builds. Turning off environment variable replacement with inlineEnvironment: false is mainly a security feature, but most of the time in apps you still want to inline NODE_ENV. It should already be possible to disable by setting inlineEnvironment: [] instead. We could do a better job documenting that.

[benoitgrelard]

What you described makes sense. For library targets, I think we should leave process.env expressions as is for environment variables that aren't defined, rather than replacing with undefined. I think we should still replace defined variables by default because libraries might use them to implement feature flags for example.

I'm not sure I understand what you mean by "not defined", does it basically mean that inlineEnvironment: false or inlineEnvironment: [] is basically like saying "don't load the variables" rather than "don't inline them"?

NODE_ENV is a bit special because many libraries use it for dev/prod builds. Turning off environment variable replacement with inlineEnvironment: false is mainly a security feature, but most of the time in apps you still want to inline NODE_ENV. It should already be possible to disable by setting inlineEnvironment: [] instead. We could do a better job documenting that.

This is the output I get now using inlineEnvironment: []:

function $cf838c15c8b009ba$export$e16d8520af44a096(a, b) {
    console.log("process.env.NAME", undefined);
    console.log("process.env.NODE_ENV", undefined);
    return a + b;
}


export {$cf838c15c8b009ba$export$e16d8520af44a096 as add};
//# sourceMappingURL=module.js.map

So it does treat NODE_ENV the same as other vars now. However this still doesn't solve our issue of maintaining the checks in the built output.

Is there anything that can be done to achieve that?

[devongovett]

I'm not sure I understand what you mean by "not defined"

Yeah, either not included because of the inlineEnvironment config, or just because it's not defined in a .env file or your shell.

Is there anything that can be done to achieve that?

We need to make some changes to library mode to make this work.

[benoitgrelard]

We need to make some changes to library mode to make this work.

Gotcha. How do you manage dev checks in react-aria, I imagine you use parcel to bundle the packages?

[devongovett]

We don't currently. Actually we recently ran into this problem as well. Might have some time to fix it soon.

[benoitgrelard]

Cool, we'll keep an eye on the issue then 👍

[benoitgrelard]

Hi @devongovett, is there any update regarding this? I'd be happy to help with some guidance if the feature isn't too difficult (I've never looked at how parcel works).

[lecstor]

for our case I think I've got a usable work-around for this..

(we're using parcel to build a package that provides utility used in webpack config and needs access to env vars during that build, not this one)

// Parcel will mess with our env references that we want persisted unless we
// make it too hard for it, so we use dynamic property lookups to access
// process.env properties
const WMF_MODULE_URL = "WMF_MODULE_URL";
const NODE_ENV = "NODE_ENV";
// eg const moduleUrl = process.env[WMF_MODULE_URL]

The following also works but I'm fairly sure the next developer that comes along would "fix" it without thinking.

const moduleUrl = process.env[`WMF_MODULE_URL`];

[danieltroger]

Also suffering from this issue, we'd need to have process.env.SOMETHING be preserved in the output.

Is there any workaround available currently?

[danieltroger]

I found a very simple workaround which is to tell parcel that we're actually building for nodeJS (even though I'm building browser code for the browser). This makes it preserve all the process.env.VARIABLE statements.

Unsure if there are any unwanted implications of that, the docs are not very specific about what it changes, but so far it seems to work:

diff --git a/package.json b/package.json
index 6829d61130..0ca822d5bd 100644
--- a/package.json
+++ b/package.json
@@ -18,7 +18,7 @@
   "targets": {
     "module": {
       "isLibrary": true,
-      "context": "browser",
+      "context": "node",
       "engines": {
         "browsers": "last 1 chrome version"
       },
@@ -26,7 +26,7 @@
     },
     "main": {
       "isLibrary": true,
-      "context": "browser",
+      "context": "node",
       "engines": {
         "browsers": "last 1 chrome version"
       },

[Mikescops]

I confirm what @danieltroger says here, it clearly solves the issue (which was hard to figure out at first).

When you build a node only package it's a bit surprising that process.env works but your process.env.VAR get replaced by undefined at compilation when you expect it to be replaced at runtime.

[awerlang]

@benoitgrelard I don't think your expected output matches the configuration. Env replace happens in console.log but not in conditionals. It should be either both or none at all.