fix: improve parsing of --env flag (#3286)

Author: snitin315

packages/webpack-cli/src/webpack-cli.ts

@@ -756,18 +756,22 @@ class WebpackCLI implements IWebpackCLI {
           value: string,
           previous: Record<string, BasicPrimitive | object> = {},
         ): Record<string, BasicPrimitive | object> => {
-          // for https://github.com/webpack/webpack-cli/issues/2642
-          if (value.endsWith("=")) {
-            value.concat('""');
-          }
-
           // This ensures we're only splitting by the first `=`
           const [allKeys, val] = value.split(/=(.+)/, 2);
           const splitKeys = allKeys.split(/\.(?!$)/);
 
           let prevRef = previous;
 
           splitKeys.forEach((someKey, index) => {
+            // https://github.com/webpack/webpack-cli/issues/3284
+            if (someKey.endsWith("=")) {
+              // remove '=' from key
+              someKey = someKey.slice(0, -1);
+              // @ts-expect-error we explicitly want to set it to undefined
+              prevRef[someKey] = undefined;
+              return;
+            }
+
             if (!prevRef[someKey]) {
               prevRef[someKey] = {};
             }

test/build/config/type/function-with-env/function-with-env.test.js

@@ -1,7 +1,7 @@
 "use strict";
 const { existsSync } = require("fs");
 const { resolve } = require("path");
-const { run, readFile } = require("../../../../utils/test-utils");
+const { run, readFile, isWindows } = require("../../../../utils/test-utils");
 
 describe("function configuration", () => {
   it("should throw when env is not supplied", async () => {
@@ -17,7 +17,7 @@ describe("function configuration", () => {
 
     expect(exitCode).toBe(0);
     expect(stderr).toBeFalsy();
-    expect(stdout).toBeTruthy();
+    expect(stdout).toContain("isProd: true");
     // Should generate the appropriate files
     expect(existsSync(resolve(__dirname, "./dist/prod.js"))).toBeTruthy();
   });
@@ -27,7 +27,7 @@ describe("function configuration", () => {
 
     expect(exitCode).toBe(0);
     expect(stderr).toBeFalsy();
-    expect(stdout).toBeTruthy();
+    expect(stdout).toContain("isDev: true");
     // Should generate the appropriate files
     expect(existsSync(resolve(__dirname, "./dist/dev.js"))).toBeTruthy();
   });
@@ -44,7 +44,8 @@ describe("function configuration", () => {
 
     expect(exitCode).toBe(0);
     expect(stderr).toBeFalsy();
-    expect(stdout).toBeTruthy();
+    expect(stdout).toContain("environment: 'production'");
+    expect(stdout).toContain("app: { title: 'Luffy' }");
     // Should generate the appropriate files
     expect(existsSync(resolve(__dirname, "./dist/Luffy.js"))).toBeTruthy();
   });
@@ -61,7 +62,7 @@ describe("function configuration", () => {
 
     expect(exitCode).toBe(0);
     expect(stderr).toBeFalsy();
-    expect(stdout).toBeTruthy();
+    expect(stdout).toContain("environment: 'production'");
     // Should generate the appropriate files
     expect(existsSync(resolve(__dirname, "./dist/Atsumu.js"))).toBeTruthy();
   });
@@ -78,7 +79,8 @@ describe("function configuration", () => {
 
     expect(exitCode).toBe(0);
     expect(stderr).toBeFalsy();
-    expect(stdout).toBeTruthy();
+    expect(stdout).toContain("environment: 'multipleq'");
+    expect(stdout).toContain("file: 'name=is=Eren'");
     // Should generate the appropriate files
     expect(existsSync(resolve(__dirname, "./dist/name=is=Eren.js"))).toBeTruthy();
   });
@@ -95,7 +97,8 @@ describe("function configuration", () => {
 
     expect(exitCode).toBe(0);
     expect(stderr).toBeFalsy();
-    expect(stdout).toBeTruthy();
+    expect(stdout).toContain("environment: 'dot'");
+    expect(stdout).toContain("'name.': 'Hisoka'");
     // Should generate the appropriate files
     expect(existsSync(resolve(__dirname, "./dist/Hisoka.js"))).toBeTruthy();
   });
@@ -112,7 +115,8 @@ describe("function configuration", () => {
 
     expect(exitCode).toBe(0);
     expect(stderr).toBeFalsy();
-    expect(stdout).toBeTruthy();
+    expect(stdout).toContain("environment: 'dot'");
+    expect(stdout).toContain("'name.': true");
     // Should generate the appropriate files
     expect(existsSync(resolve(__dirname, "./dist/true.js"))).toBeTruthy();
   });
@@ -122,7 +126,7 @@ describe("function configuration", () => {
 
     expect(exitCode).toBe(0);
     expect(stderr).toBeFalsy();
-    expect(stdout).toBeTruthy();
+    expect(stdout).toContain(`foo: "''"`);
     // Should generate the appropriate files
     expect(existsSync(resolve(__dirname, "./dist/empty-string.js"))).toBeTruthy();
   });
@@ -132,7 +136,7 @@ describe("function configuration", () => {
 
     expect(exitCode).toBe(0);
     expect(stderr).toBeFalsy();
-    expect(stdout).toBeTruthy();
+    expect(stdout).toContain(`foo: "bar=''"`);
     // Should generate the appropriate files
     expect(existsSync(resolve(__dirname, "./dist/new-empty-string.js"))).toBeTruthy();
   });
@@ -142,11 +146,59 @@ describe("function configuration", () => {
 
     expect(exitCode).toBe(0);
     expect(stderr).toBeFalsy();
-    expect(stdout).toBeTruthy();
+    // should log foo: undefined
+    expect(stdout).toContain("foo: undefined");
+  });
+
+  it('Supports env variable with "foo=undefined" at the end', async () => {
+    const { exitCode, stderr, stdout } = await run(__dirname, ["--env", `foo=undefined`]);
+
+    expect(exitCode).toBe(0);
+    expect(stderr).toBeFalsy();
+    // should log foo: 'undefined'
+    expect(stdout).toContain("foo: 'undefined'");
     // Should generate the appropriate files
-    expect(existsSync(resolve(__dirname, "./dist/equal-at-the-end.js"))).toBeTruthy();
+    expect(existsSync(resolve(__dirname, "./dist/undefined-foo.js"))).toBeTruthy();
   });
 
+  // macOS/Linux specific syntax
+  if (!isWindows) {
+    it("Supports empty string in shell environment", async () => {
+      const { exitCode, stderr, stdout } = await run(__dirname, ["--env", "foo=\\'\\'"], {
+        shell: true,
+      });
+
+      expect(exitCode).toBe(0);
+      expect(stderr).toBeFalsy();
+      expect(stdout).toContain(`foo: "''"`);
+      // Should generate the appropriate files
+      expect(existsSync(resolve(__dirname, "./dist/empty-string.js"))).toBeTruthy();
+    });
+    it("should set the variable to undefined if empty string is not escaped in shell environment", async () => {
+      const { exitCode, stderr, stdout } = await run(__dirname, ["--env", "foo=''"], {
+        shell: true,
+      });
+
+      expect(exitCode).toBe(0);
+      expect(stderr).toBeFalsy();
+      expect(stdout).toContain(`foo: undefined`);
+    });
+    it('Supports env variable with "=$NON_EXISTENT_VAR" at the end', async () => {
+      const { exitCode, stderr, stdout } = await run(
+        __dirname,
+        ["--env", `foo=$NON_EXISTENT_VAR`],
+        {
+          shell: true,
+        },
+      );
+
+      expect(exitCode).toBe(0);
+      expect(stderr).toBeFalsy();
+      // should log foo: undefined
+      expect(stdout).toContain("foo: undefined");
+    });
+  }
+
   it("is able to understand multiple env flags", async () => {
     const { exitCode, stderr, stdout } = await run(__dirname, [
       "--env",
@@ -159,7 +211,8 @@ describe("function configuration", () => {
 
     expect(exitCode).toBe(0);
     expect(stderr).toBeFalsy();
-    expect(stdout).toBeTruthy();
+    expect(stdout).toContain("verboseStats: true");
+    expect(stdout).toContain("envMessage: true");
     // check that the verbose env is respected
     expect(stdout).toContain("LOG from webpack");
 
@@ -189,7 +242,7 @@ describe("function configuration", () => {
 
     expect(exitCode).toBe(0);
     expect(stderr).toBeFalsy();
-    expect(stdout).toBeTruthy();
+    expect(stdout).toContain("'name.': 'baz'");
     // Should generate the appropriate files
     expect(existsSync(resolve(__dirname, "./dist/baz.js"))).toBeTruthy();
   });

test/build/config/type/function-with-env/webpack.config.js

@@ -1,6 +1,7 @@
 const { DefinePlugin } = require("webpack");
 
 module.exports = (env) => {
+  console.log(env);
   if (env.isProd) {
     return {
       entry: "./a.js",
@@ -25,11 +26,11 @@ module.exports = (env) => {
       },
     };
   }
-  if (env["foo="]) {
+  if (env.foo === "undefined") {
     return {
       entry: "./a.js",
       output: {
-        filename: "equal-at-the-end.js",
+        filename: "undefined-foo.js",
       },
     };
   }

test/build/config/type/function-with-env/webpack.env.config.js

@@ -1,4 +1,5 @@
 module.exports = (env) => {
+  console.log(env);
   const { environment, app, file } = env;
   const customName = file && file.name && file.name.is && file.name.is.this;
   const appTitle = app && app.title;