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

EnvironmentPlugin doesn't error if environment variable wasn't found? #7448

Closed
fasiha opened this issue Oct 30, 2024 · 3 comments · Fixed by #7461
Closed

EnvironmentPlugin doesn't error if environment variable wasn't found? #7448

fasiha opened this issue Oct 30, 2024 · 3 comments · Fixed by #7461
Assignees

Comments

@fasiha
Copy link

fasiha commented Oct 30, 2024

Bug report

What is the current behavior?

Webpack configured to use EnvironmentPlugin will happily build code with unknown environment variables like process.env.UNKNOWN.

If the current behavior is a bug, please provide the steps to reproduce.

I have created a fork of the webpack repo and tweaked an example to demonstrate the issue at https://github.com/fasiha/webpack/tree/example-environment-plugin. You can either check out that fork and branch and then run

yarn && yarn setup && yarn add --dev webpack-cli
cd examples/multi-compiler
node build.js

or simply check out the current main branch and apply the following patch to modify one of the examples:

diff --git a/examples/multi-compiler/example.js b/examples/multi-compiler/example.js
index 7a10193a2..ac996212d 100644
--- a/examples/multi-compiler/example.js
+++ b/examples/multi-compiler/example.js
@@ -1,4 +1,4 @@
-if(ENV === "mobile") {
+if (process.env.TESTING && process.env.ENV === "mobile") {
 	require("./mobile-stuff");
 }
-console.log("Running " + ENV + " build");
\ No newline at end of file
+console.log("Running " + process.env.ENV + " build");
diff --git a/examples/multi-compiler/webpack.config.js b/examples/multi-compiler/webpack.config.js
index e7b01428c..0de1ae133 100644
--- a/examples/multi-compiler/webpack.config.js
+++ b/examples/multi-compiler/webpack.config.js
@@ -11,8 +11,8 @@ module.exports = [
 			filename: "mobile.js"
 		},
 		plugins: [
-			new webpack.DefinePlugin({
-				ENV: JSON.stringify("mobile")
+			new webpack.EnvironmentPlugin({
+				ENV: "mobile"
 			})
 		]
 	},
@@ -26,8 +26,8 @@ module.exports = [
 			filename: "desktop.js"
 		},
 		plugins: [
-			new webpack.DefinePlugin({
-				ENV: JSON.stringify("desktop")
+			new webpack.EnvironmentPlugin({
+				ENV: "desktop"
 			})
 		]
 	}

Then run cd examples/multi-compiler && node build.js.

Whether you use my branch or the above patch, building the modified multi-compiler example will happily run without any errors.

The resulting code has process.env.ENV correctly interpolated to desktop or mobile per Webpack config, but the new unknown process.env.TESTING is erroneously left intact.

What is the expected behavior?

I expect building multi-compiler in the above branch (or the current webpack main with the patch applied) to fail per EnvironmentPlugin docs:

If an environment variable is not found during bundling and no default value was provided, webpack will throw an error instead of a warning.

Other relevant information:
webpack version: main branch 2e174de66
Node.js version: v20.18.0
Operating System: macOS and Rocky9 Linux
Additional tools:

@alexander-akait
Copy link
Member

alexander-akait commented Oct 31, 2024

If an environment variable is not found during bundling and no default value was provided, webpack will throw an error instead of a warning.

This is a documentation issue.

It means we use this code to get value:

const value =
  process.env[key] !== undefined
    ? process.env[key]
    : this.defaultValues[key];

And if you have new webpack.EnvironmentPlugin(["ENV"]), it will throw an error when you don't have it in process.env, we never threw an error if we found the value in the source code.

I would recommend using a linter in this case if you want to prohibit the use of some values in the source code (in process.env).

In your case you can use

new webpack.EnvironmentPlugin(["ENV", "TESTING"])

or

new webpack.EnvironmentPlugin({
  ENV: "desktop",
  TESTING: undefined
})`  

by using the object you provide default values

@alexander-akait alexander-akait transferred this issue from webpack/webpack Oct 31, 2024
@fasiha
Copy link
Author

fasiha commented Oct 31, 2024

Understood, thanks, sorry for misunderstanding! It was very surprising to see process.env.… in the built output but I now see that this is intended behavior.

If it's ok with you, I'll prepare a PR explaining this a bit more in the documentation?

@alexander-akait
Copy link
Member

@fasiha Yeah, feel free to send a pr ❤️

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 a pull request may close this issue.

3 participants