feat!: differentiate between --node-env and `--define-process-env-n…

…ode-env`
webpack · Nov 5, 2024 · 759901b · 759901b
1 parent 8d9e5c0
commit 759901b
Show file tree

Hide file tree

Showing 20 changed files with 160 additions and 92 deletions.
diff --git a/packages/webpack-cli/src/webpack-cli.ts b/packages/webpack-cli/src/webpack-cli.ts
@@ -971,7 +971,8 @@ class WebpackCLI implements IWebpackCLI {
           },
         ],
         multiple: false,
-        description: "Sets process.env.NODE_ENV to the specified value.",
+        description:
+          "Sets process.env.NODE_ENV before loading the config, making it accessible within the configuration.",
         helpLevel: "minimum",
       },
       {
@@ -983,7 +984,7 @@ class WebpackCLI implements IWebpackCLI {
         ],
         multiple: false,
         description:
-          "Sets process.env.NODE_ENV to the specified value. (Currently an alias for `--node-env`).",
+          "Sets process.env.NODE_ENV after loading the config, so it’s only available at runtime, not in the configuration.",
         helpLevel: "verbose",
       },
 
@@ -1645,9 +1646,14 @@ class WebpackCLI implements IWebpackCLI {
           );
 
           if (possibleValues.length > 0) {
-            this.logger.raw(
-              `${bold("Possible values:")} ${JSON.stringify(possibleValues.join(" | "))}`,
-            );
+            // Convert the possible values to a union type string
+            // ['mode', 'development', 'production'] => "'mode' | 'development' | 'production'"
+            // [false, 'eval'] => "false | 'eval'"
+            const possibleValuesUnionTypeString = possibleValues
+              .map((value) => (typeof value === "string" ? `'${value}'` : value))
+              .join(" | ");
+
+            this.logger.raw(`${bold("Possible values:")} ${possibleValuesUnionTypeString}`);
           }
         }
 
@@ -2358,16 +2364,30 @@ class WebpackCLI implements IWebpackCLI {
     options: Partial<WebpackDevServerOptions>,
     callback?: Callback<[Error | undefined, WebpackCLIStats | undefined]>,
   ): Promise<WebpackCompiler> {
-    if (typeof options.defineProcessEnvNodeEnv === "string") {
-      // TODO: This should only set NODE_ENV for the runtime not for the config too. Change this during next breaking change.
-      process.env.NODE_ENV = options.defineProcessEnvNodeEnv;
-    } else if (typeof options.nodeEnv === "string") {
+    const isNodeEnvOptionDefined = typeof options.nodeEnv === "string";
+    const isDefineProcessEnvNodeEnvOptionDefined =
+      typeof options.defineProcessEnvNodeEnv === "string";
+
+    if (isNodeEnvOptionDefined && isDefineProcessEnvNodeEnvOptionDefined) {
+      this.logger.error(
+        "You can't use both '--node-env' and '--define-process-env-node-env' options at the same time.",
+      );
+      process.exit(2);
+    }
+
+    // Setting before loading the config
+    if (isNodeEnvOptionDefined) {
       process.env.NODE_ENV = options.nodeEnv;
     }
 
     let config = await this.loadConfig(options);
     config = await this.buildConfig(config, options);
 
+    // Setting after loading the config
+    if (isDefineProcessEnvNodeEnvOptionDefined) {
+      process.env.NODE_ENV = options.defineProcessEnvNodeEnv;
+    }
+
     let compiler: WebpackCompiler;
 
     try {

diff --git a/test/build/define-process-env-node-env/auto-mode.config.js b/test/build/define-process-env-node-env/auto-mode.config.js
diff --git a/test/build/define-process-env-node-env/define-process-env-node-env.test.js b/test/build/define-process-env-node-env/define-process-env-node-env.test.js
@@ -3,6 +3,21 @@
 const { run } = require("../../utils/test-utils");
 
 describe("--define-process-env-node-env flag", () => {
+  const OLD_ENV = process.env;
+
+  beforeEach(() => {
+    jest.resetModules();
+    process.env = { ...OLD_ENV };
+    // Ensure NODE_ENV is undefined before running the test
+    if (process.env.NODE_ENV) {
+      delete process.env.NODE_ENV;
+    }
+  });
+
+  afterAll(() => {
+    process.env = OLD_ENV;
+  });
+
   it('should set "process.env.NODE_ENV" to "development"', async () => {
     const { exitCode, stderr, stdout } = await run(__dirname, [
       "--define-process-env-node-env",
@@ -11,7 +26,8 @@ describe("--define-process-env-node-env flag", () => {
 
     expect(exitCode).toBe(0);
     expect(stderr).toBeFalsy();
-    expect(stdout).toContain("mode: 'development'");
+    expect(stdout).toContain("[webpack.config.js] process.env.NODE_ENV: undefined");
+    expect(stdout).toContain("[WebpackCLITestPlugin] process.env.NODE_ENV: development");
   });
 
   it('should set "process.env.NODE_ENV" to "production"', async () => {
@@ -22,7 +38,8 @@ describe("--define-process-env-node-env flag", () => {
 
     expect(exitCode).toBe(0);
     expect(stderr).toBeFalsy();
-    expect(stdout).toContain("mode: 'production'");
+    expect(stdout).toContain("[webpack.config.js] process.env.NODE_ENV: undefined");
+    expect(stdout).toContain("[WebpackCLITestPlugin] process.env.NODE_ENV: production");
   });
 
   it('should set "process.env.NODE_ENV" to "none"', async () => {
@@ -33,45 +50,22 @@ describe("--define-process-env-node-env flag", () => {
 
     expect(exitCode).toBe(0);
     expect(stderr).toBeFalsy();
-    expect(stdout).toContain("mode: 'none'");
+    expect(stdout).toContain("[webpack.config.js] process.env.NODE_ENV: undefined");
+    expect(stdout).toContain("[WebpackCLITestPlugin] process.env.NODE_ENV: none");
   });
 
-  it('should set "process.env.NODE_ENV" and the "mode" option to "development"', async () => {
+  it('should throw error if both "--node-env" and "--define-process-env-node-env" are passed', async () => {
     const { exitCode, stderr, stdout } = await run(__dirname, [
-      "--define-process-env-node-env",
+      "--node-env",
       "development",
-      "--config",
-      "./auto-mode.config.js",
-    ]);
-
-    expect(exitCode).toBe(0);
-    expect(stderr).toBeFalsy();
-    expect(stdout).toContain("mode: 'development'");
-  });
-
-  it('should set "process.env.NODE_ENV" and the "mode" option to "production"', async () => {
-    const { exitCode, stderr, stdout } = await run(__dirname, [
       "--define-process-env-node-env",
       "production",
-      "--config",
-      "./auto-mode.config.js",
-    ]);
-
-    expect(exitCode).toBe(0);
-    expect(stderr).toBeFalsy();
-    expect(stdout).toContain("mode: 'production'");
-  });
-
-  it('should set "process.env.NODE_ENV" and the "mode" option to "none"', async () => {
-    const { exitCode, stderr, stdout } = await run(__dirname, [
-      "--define-process-env-node-env",
-      "none",
-      "--config",
-      "./auto-mode.config.js",
     ]);
 
-    expect(exitCode).toBe(0);
-    expect(stderr).toBeFalsy();
-    expect(stdout).toContain("mode: 'none'");
+    expect(exitCode).toBe(2);
+    expect(stderr).toContain(
+      "You can't use both '--node-env' and '--define-process-env-node-env' options at the same time.",
+    );
+    expect(stdout).toBeFalsy();
   });
 });
diff --git a/test/build/define-process-env-node-env/webpack.config.js b/test/build/define-process-env-node-env/webpack.config.js
@@ -1,6 +1,8 @@
 const WebpackCLITestPlugin = require("../../utils/webpack-cli-test-plugin");
 
+console.log("[webpack.config.js] process.env.NODE_ENV:", process.env.NODE_ENV);
+
 module.exports = {
-  mode: process.env.NODE_ENV,
-  plugins: [new WebpackCLITestPlugin()],
+  mode: "none",
+  plugins: [new WebpackCLITestPlugin(null, { showAll: false, envVariables: ["NODE_ENV"] })],
 };