fix: fix panic when optional chain partially replaced with defines (#…

…3554)
evanw · Dec 18, 2023 · accfb14 · accfb14
1 parent b76b54f
commit accfb14
Show file tree

Hide file tree

Showing 3 changed files with 100 additions and 3 deletions.
diff --git a/internal/bundler_tests/bundler_default_test.go b/internal/bundler_tests/bundler_default_test.go
@@ -5329,6 +5329,64 @@ func TestDefineOptionalChainLowered(t *testing.T) {
 	})
 }
 
+// See: https://github.com/evanw/esbuild/issues/3551
+func TestDefineOptionalChainPanicIssue3551(t *testing.T) {
+	defines := config.ProcessDefines(map[string]config.DefineData{
+		"x": {
+			DefineExpr: &config.DefineExpr{
+				Constant: &js_ast.EObject{},
+			},
+		},
+		"a.b": {
+			DefineExpr: &config.DefineExpr{
+				Constant: &js_ast.EObject{},
+			},
+		},
+	})
+	default_suite.expectBundled(t, bundled{
+		files: map[string]string{
+			"/id-define.js": `
+				x?.y.z;
+				(x?.y).z;
+				x?.y["z"];
+				(x?.y)["z"];
+				x?.y();
+				(x?.y)();
+				x?.y.z();
+				(x?.y).z();
+				x?.y["z"]();
+				(x?.y)["z"]();
+				delete x?.y.z;
+				delete (x?.y).z;
+				delete x?.y["z"];
+				delete (x?.y)["z"];
+			`,
+			"/dot-define.js": `
+				a?.b.c;
+				(a?.b).c;
+				a?.b["c"];
+				(a?.b)["c"];
+				a?.b();
+				(a?.b)();
+				a?.b.c();
+				(a?.b).c();
+				a?.b["c"]();
+				(a?.b)["c"]();
+				delete a?.b.c;
+				delete (a?.b).c;
+				delete a?.b["c"];
+				delete (a?.b)["c"];
+			`,
+		},
+		entryPaths: []string{"/id-define.js", "/dot-define.js"},
+		options: config.Options{
+			Mode:         config.ModeBundle,
+			AbsOutputDir: "/out",
+			Defines:      &defines,
+		},
+	})
+}
+
 // See: https://github.com/evanw/esbuild/issues/2407
 func TestDefineInfiniteLoopIssue2407(t *testing.T) {
 	defines := config.ProcessDefines(map[string]config.DefineData{

diff --git a/internal/bundler_tests/snapshots/snapshots_default.txt b/internal/bundler_tests/snapshots/snapshots_default.txt
@@ -1081,6 +1081,42 @@ console.log([
   (_a = a[b]) == null ? void 0 : _a[c]
 ]);
 
+================================================================================
+TestDefineOptionalChainPanicIssue3551
+---------- /out/id-define.js ----------
+// id-define.js
+({})?.y.z;
+({}?.y).z;
+({})?.y["z"];
+({}?.y)["z"];
+({})?.y();
+({}?.y)();
+({})?.y.z();
+({}?.y).z();
+({})?.y["z"]();
+({}?.y)["z"]();
+delete {}?.y.z;
+delete ({}?.y).z;
+delete {}?.y["z"];
+delete ({}?.y)["z"];
+
+---------- /out/dot-define.js ----------
+// dot-define.js
+({}).c;
+({}).c;
+({})["c"];
+({})["c"];
+({})();
+({})();
+({}).c();
+({}).c();
+({})["c"]();
+({})["c"]();
+delete {}.c;
+delete {}.c;
+delete {}["c"];
+delete {}["c"];
+
 ================================================================================
 TestDefineThis
 ---------- /out.js ----------

diff --git a/internal/js_parser/js_parser.go b/internal/js_parser/js_parser.go
@@ -13462,7 +13462,8 @@ func (p *parser) visitExprInOut(expr js_ast.Expr, in exprIn) (js_ast.Expr, exprO
 		}
 
 		// Lower optional chaining if we're the top of the chain
-		containsOptionalChain := e.OptionalChain != js_ast.OptionalChainNone
+		containsOptionalChain := e.OptionalChain == js_ast.OptionalChainStart ||
+			(e.OptionalChain == js_ast.OptionalChainContinue && out.childContainsOptionalChain)
 		if containsOptionalChain && !in.hasChainParent {
 			return p.lowerOptionalChain(expr, in, out)
 		}
@@ -13620,7 +13621,8 @@ func (p *parser) visitExprInOut(expr js_ast.Expr, in exprIn) (js_ast.Expr, exprO
 		}
 
 		// Lower optional chaining if we're the top of the chain
-		containsOptionalChain := e.OptionalChain != js_ast.OptionalChainNone
+		containsOptionalChain := e.OptionalChain == js_ast.OptionalChainStart ||
+			(e.OptionalChain == js_ast.OptionalChainContinue && out.childContainsOptionalChain)
 		if containsOptionalChain && !in.hasChainParent {
 			return p.lowerOptionalChain(expr, in, out)
 		}
@@ -14718,7 +14720,8 @@ func (p *parser) visitExprInOut(expr js_ast.Expr, in exprIn) (js_ast.Expr, exprO
 		}
 
 		// Lower optional chaining if we're the top of the chain
-		containsOptionalChain := e.OptionalChain != js_ast.OptionalChainNone
+		containsOptionalChain := e.OptionalChain == js_ast.OptionalChainStart ||
+			(e.OptionalChain == js_ast.OptionalChainContinue && out.childContainsOptionalChain)
 		if containsOptionalChain && !in.hasChainParent {
 			return p.lowerOptionalChain(expr, in, out)
 		}