fix #4348: bundler bug with var inside if

evanw · evanw · commit 7a72735c0d5e · 2025-12-03T12:56:16.000-05:00
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -2,6 +2,10 @@
 
 ## Unreleased
 
+* Fix bundler bug with `var` nested inside `if` ([#4348](https://github.com/evanw/esbuild/issues/4348))
+
+    This release fixes a bug with the bundler that happens when importing an ES module using `require` (which causes it to be wrapped) and there's a top-level `var` inside an `if` statement without being wrapped in a `{ ... }` block (and a few other conditions). The bundling transform needed to hoist these `var` declarations outside of the lazy ES module wrapper for correctness. See the issue for details.
+
 * Fix minifier bug with `for` inside `try` inside label ([#4351](https://github.com/evanw/esbuild/issues/4351))
 
     This fixes an old regression from [version v0.21.4](https://github.com/evanw/esbuild/releases/v0.21.4). Some code was introduced to move the label inside the `try` statement to address a problem with transforming labeled `for await` loops to avoid the `await` (the transformation involves converting the `for await` loop into a `for` loop and wrapping it in a `try` statement). However, it introduces problems for cross-compiled JVM code that uses all three of these features heavily. This release restricts this transform to only apply to `for` loops that esbuild itself generates internally as part of the `for await` transform. Here is an example of some affected code:
diff --git a/internal/bundler_tests/bundler_default_test.go b/internal/bundler_tests/bundler_default_test.go
@@ -9358,3 +9358,32 @@ func TestResolveExtensionsOrderIssue4053(t *testing.T) {
 		},
 	})
 }
+
+func TestBundleESMWithNestedVarIssue4348(t *testing.T) {
+	css_suite.expectBundled(t, bundled{
+		files: map[string]string{
+			"/entry.js": `
+				require('./foo')
+			`,
+			"/foo.js": `
+				var a = 'a'
+				for (var b = 'b'; 0; ) ;
+				if (true) { var c = 'c' }
+				if (true) var d = 'd'
+				if (false) {} else var e = 'e'
+				var x = 1
+				while (x--) var f = 'f'
+				do var g = 'g'; while (0);
+				for (; x++; ) var h = 'h'
+				for (var y in 'y') var i = 'i'
+				for (var y of 'y') var j = 'j'
+				export { a, b, c, d, e, f, g, h, i, j }
+			`,
+		},
+		entryPaths: []string{"/entry.js"},
+		options: config.Options{
+			Mode:          config.ModeBundle,
+			AbsOutputFile: "/out.js",
+		},
+	})
+}
diff --git a/internal/bundler_tests/snapshots/snapshots_css.txt b/internal/bundler_tests/snapshots/snapshots_css.txt
@@ -13,6 +13,48 @@ a {
   background: url(data:application/octet-stream;base64,iVBORw0KGgo=);
 }
 
+================================================================================
+TestBundleESMWithNestedVarIssue4348
+---------- /out.js ----------
+// foo.js
+var foo_exports = {};
+__export(foo_exports, {
+  a: () => a,
+  b: () => b,
+  c: () => c,
+  d: () => d,
+  e: () => e,
+  f: () => f,
+  g: () => g,
+  h: () => h,
+  i: () => i,
+  j: () => j
+});
+var a, b, c, d, e, x, f, g, h, i, y, j, y;
+var init_foo = __esm({
+  "foo.js"() {
+    a = "a";
+    for (b = "b"; 0; ) ;
+    if (true) {
+      c = "c";
+    }
+    if (true) d = "d";
+    if (false) {
+    } else e = "e";
+    x = 1;
+    while (x--) f = "f";
+    do
+      g = "g";
+    while (0);
+    for (; x++; ) h = "h";
+    for (y in "y") i = "i";
+    for (y of "y") j = "j";
+  }
+});
+
+// entry.js
+init_foo();
+
 ================================================================================
 TestCSSAndJavaScriptCodeSplittingIssue1064
 ---------- /out/a.js ----------
diff --git a/internal/js_parser/js_parser.go b/internal/js_parser/js_parser.go
@@ -252,6 +252,7 @@ type parser struct {
 	firstJSXElementLoc     logger.Loc
 
 	fnOrArrowDataVisit fnOrArrowDataVisit
+	singleStmtDepth    int
 
 	// ArrowFunction is a special case in the grammar. Although it appears to be
 	// a PrimaryExpression, it's actually an AssignmentExpression. This means if
@@ -9911,7 +9912,9 @@ func (p *parser) visitSingleStmt(stmt js_ast.Stmt, kind stmtsKind) js_ast.Stmt {
 		}
 	}
 
+	p.singleStmtDepth++
 	stmts := p.visitStmts([]js_ast.Stmt{stmt}, kind)
+	p.singleStmtDepth--
 
 	// Balance the fake block scope introduced above
 	if hasIfScope {
@@ -11623,7 +11626,7 @@ const (
 // instead of having to traverse recursively into the statement tree.
 func (p *parser) maybeRelocateVarsToTopLevel(decls []js_ast.Decl, mode relocateVarsMode) (js_ast.Stmt, bool) {
 	// Only do this when bundling, and not when the scope is already top-level
-	if p.options.mode != config.ModeBundle || p.currentScope == p.moduleScope {
+	if p.options.mode != config.ModeBundle || (p.currentScope == p.moduleScope && p.singleStmtDepth == 0) {
 		return js_ast.Stmt{}, false
 	}
 
diff --git a/scripts/end-to-end-tests.js b/scripts/end-to-end-tests.js
@@ -1906,6 +1906,28 @@ for (const minify of [[], ['--minify']]) {
         'in.js': `import foo from './foo'; if (foo() !== (function() { return this })()) throw 'fail'`,
         'foo.js': `module.exports = function() { return this }`,
       }),
+
+      // https://github.com/evanw/esbuild/issues/4348
+      test(['--bundle', '--format=cjs', 'in.js', '--outfile=node.js', '--target=' + target].concat(minify), {
+        'in.js': `
+          var foo = Object.entries({ ...require('./foo') }).join('|')
+          if (foo !== 'a,a|b,b|c,c|d,d|e,e|f,f|g,g|h,h|i,i|j,j') throw 'fail: ' + foo
+        `,
+        'foo.js': `
+          var a = 'a'
+          for (var b = 'b'; 0; ) ;
+          if (true) { var c = 'c' }
+          if (true) var d = 'd'
+          if (false) {} else var e = 'e'
+          var x = 1
+          while (x--) var f = 'f'
+          do var g = 'g'; while (0);
+          for (; x++; ) var h = 'h'
+          for (var y in 'y') var i = 'i'
+          for (var y ${target === 'es5' ? 'in' : 'of'} 'y') var j = 'j'
+          export { a, b, c, d, e, f, g, h, i, j }
+        `,
+      }),
     )
   }
 
