fix #2324: allow define to match optional chains

evanw · Jun 17, 2022 · bb97fc2 · bb97fc2
1 parent 15fde33
commit bb97fc2
Show file tree

Hide file tree

Showing 4 changed files with 129 additions and 4 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,5 +1,24 @@
 # Changelog
 
+## Unreleased
+
+* Allow `define` to match optional chain expressions ([#2324](https://github.com/evanw/esbuild/issues/2324))
+
+    Previously esbuild's `define` feature only matched member expressions that did not use optional chaining. With this release, esbuild will now also match those that use optional chaining:
+
+    ```js
+    // Original code
+    console.log(a.b, a?.b)
+
+    // Old output (with --define:a.b=c)
+    console.log(c, a?.b);
+
+    // New output (with --define:a.b=c)
+    console.log(c, c);
+    ```
+
+    This is for compatibility with Webpack's [`DefinePlugin`](https://webpack.js.org/plugins/define-plugin/), which behaves the same way.
+
 ## 0.14.45
 
 * Add a log message for ambiguous re-exports ([#2322](https://github.com/evanw/esbuild/issues/2322))

diff --git a/internal/bundler/bundler_default_test.go b/internal/bundler/bundler_default_test.go
@@ -4454,6 +4454,77 @@ func TestDefineThis(t *testing.T) {
 	})
 }
 
+func TestDefineOptionalChain(t *testing.T) {
+	defines := config.ProcessDefines(map[string]config.DefineData{
+		"a.b.c": {
+			DefineExpr: &config.DefineExpr{
+				Constant: &js_ast.ENumber{Value: 1},
+			},
+		},
+	})
+	default_suite.expectBundled(t, bundled{
+		files: map[string]string{
+			"/entry.js": `
+				console.log([
+					a.b.c,
+					a?.b.c,
+					a.b?.c,
+				], [
+					a['b']['c'],
+					a?.['b']['c'],
+					a['b']?.['c'],
+				], [
+					a[b][c],
+					a?.[b][c],
+					a[b]?.[c],
+				])
+			`,
+		},
+		entryPaths: []string{"/entry.js"},
+		options: config.Options{
+			Mode:          config.ModeBundle,
+			AbsOutputFile: "/out.js",
+			Defines:       &defines,
+		},
+	})
+}
+
+func TestDefineOptionalChainLowered(t *testing.T) {
+	defines := config.ProcessDefines(map[string]config.DefineData{
+		"a.b.c": {
+			DefineExpr: &config.DefineExpr{
+				Constant: &js_ast.ENumber{Value: 1},
+			},
+		},
+	})
+	default_suite.expectBundled(t, bundled{
+		files: map[string]string{
+			"/entry.js": `
+				console.log([
+					a.b.c,
+					a?.b.c,
+					a.b?.c,
+				], [
+					a['b']['c'],
+					a?.['b']['c'],
+					a['b']?.['c'],
+				], [
+					a[b][c],
+					a?.[b][c],
+					a[b]?.[c],
+				])
+			`,
+		},
+		entryPaths: []string{"/entry.js"},
+		options: config.Options{
+			Mode:                  config.ModeBundle,
+			AbsOutputFile:         "/out.js",
+			Defines:               &defines,
+			UnsupportedJSFeatures: compat.OptionalChain,
+		},
+	})
+}
+
 func TestKeepNamesTreeShaking(t *testing.T) {
 	default_suite.expectBundled(t, bundled{
 		files: map[string]string{

diff --git a/internal/bundler/snapshots/snapshots_default.txt b/internal/bundler/snapshots/snapshots_default.txt
@@ -414,6 +414,43 @@ console.log(import_meta.y);
 
 ---------- /out/dead-code.js ----------
 
+================================================================================
+TestDefineOptionalChain
+---------- /out.js ----------
+// entry.js
+console.log([
+  1,
+  1,
+  1
+], [
+  1,
+  1,
+  1
+], [
+  a[b][c],
+  a?.[b][c],
+  a[b]?.[c]
+]);
+
+================================================================================
+TestDefineOptionalChainLowered
+---------- /out.js ----------
+// entry.js
+var _a;
+console.log([
+  1,
+  1,
+  1
+], [
+  1,
+  1,
+  1
+], [
+  a[b][c],
+  a == null ? void 0 : a[b][c],
+  (_a = a[b]) == null ? void 0 : _a[c]
+]);
+
 ================================================================================
 TestDefineThis
 ---------- /out.js ----------

diff --git a/internal/js_parser/js_parser.go b/internal/js_parser/js_parser.go
@@ -10601,17 +10601,15 @@ func (p *parser) isDotOrIndexDefineMatch(expr js_ast.Expr, parts []string) bool
 		if len(parts) > 1 {
 			// Intermediates must be dot expressions
 			last := len(parts) - 1
-			return e.OptionalChain == js_ast.OptionalChainNone && parts[last] == e.Name &&
-				p.isDotOrIndexDefineMatch(e.Target, parts[:last])
+			return parts[last] == e.Name && p.isDotOrIndexDefineMatch(e.Target, parts[:last])
 		}
 
 	case *js_ast.EIndex:
 		if len(parts) > 1 {
 			if str, ok := e.Index.Data.(*js_ast.EString); ok {
 				// Intermediates must be dot expressions
 				last := len(parts) - 1
-				return e.OptionalChain == js_ast.OptionalChainNone && parts[last] == helpers.UTF16ToString(str.Value) &&
-					p.isDotOrIndexDefineMatch(e.Target, parts[:last])
+				return parts[last] == helpers.UTF16ToString(str.Value) && p.isDotOrIndexDefineMatch(e.Target, parts[:last])
 			}
 		}