fix #2761: avoid syntax error with direct eval

CHANGELOG.md

@@ -1,5 +1,37 @@
 # Changelog
 
+## Unreleased
+
+* Avoid a syntax error in the presence of direct `eval` ([#2761](https://github.com/evanw/esbuild/issues/2761))
+
+    The behavior of nested `function` declarations in JavaScript depends on whether the code is run in strict mode or not. It would be problematic if esbuild preserved nested `function` declarations in its output because then the behavior would depend on whether the output was run in strict mode or not instead of respecting the strict mode behavior of the original source code. To avoid this, esbuild transforms nested `function` declarations to preserve the intended behavior of the original source code regardless of whether the output is run in strict mode or not:
+
+    ```js
+    // Original code
+    if (true) {
+      function foo() {}
+      console.log(!!foo)
+      foo = null
+      console.log(!!foo)
+    }
+    console.log(!!foo)
+
+    // Transformed code
+    if (true) {
+      let foo2 = function() {
+      };
+      var foo = foo2;
+      console.log(!!foo2);
+      foo2 = null;
+      console.log(!!foo2);
+    }
+    console.log(!!foo);
+    ```
+
+    In the above example, the original code should print `true false true` because it's not run in strict mode (it doesn't contain `"use strict"` and is not an ES module). The code that esbuild generates has been transformed such that it prints `true false true` regardless of whether it's run in strict mode or not.
+
+    However, this transformation is impossible if the code contains direct `eval` because direct `eval` "poisons" all containing scopes by preventing anything in those scopes from being renamed. That prevents esbuild from splitting up accesses to `foo` into two separate variables with different names. Previously esbuild still did this transformation but with two variables both named `foo`, which is a syntax error. With this release esbuild will now skip doing this transformation when direct `eval` is present to avoid generating code with a syntax error. This means that the generated code may no longer behave as intended since the behavior depends on the run-time strict mode setting instead of the strict mode setting present in the original source code. To fix this problem, you will need to remove the use of direct `eval`.
+
 ## 0.16.10
 
 * Change the default "legal comment" behavior again ([#2745](https://github.com/evanw/esbuild/issues/2745))

internal/js_parser/js_parser.go

@@ -7827,6 +7827,25 @@ func (p *parser) visitStmts(stmts []js_ast.Stmt, kind stmtsKind) []js_ast.Stmt {
 				nonFnStmts = append(nonFnStmts, stmt)
 				continue
 			}
+
+			// This transformation of function declarations in nested scopes is
+			// intended to preserve the hoisting semantics of the original code. In
+			// JavaScript, function hoisting works differently in strict mode vs.
+			// sloppy mode code. We want the code we generate to use the semantics of
+			// the original environment, not the generated environment. However, if
+			// direct "eval" is present then it's not possible to preserve the
+			// semantics because we need two identifiers to do that and direct "eval"
+			// means neither identifier can be renamed to something else. So in that
+			// case we give up and do not preserve the semantics of the original code.
+			if p.currentScope.ContainsDirectEval {
+				if hoistedRef, ok := p.hoistedRefForSloppyModeBlockFn[s.Fn.Name.Ref]; ok {
+					// Merge the two identifiers back into a single one
+					p.symbols[hoistedRef.InnerIndex].Link = s.Fn.Name.Ref
+				}
+				nonFnStmts = append(nonFnStmts, stmt)
+				continue
+			}
+
 			index, ok := fnStmts[s.Fn.Name.Ref]
 			if !ok {
 				index = len(letDecls)

internal/js_parser/js_parser_test.go

@@ -1457,6 +1457,16 @@ func TestLexicalDecl(t *testing.T) {
 	expectParseError(t, "with (1) label: label2: function f() {}", "<stdin>: ERROR: Cannot use a declaration in a single-statement context\n")
 	expectParseError(t, "while (1) label: label2: function f() {}", "<stdin>: ERROR: Cannot use a declaration in a single-statement context\n")
 	expectParseError(t, "do label: label2: function f() {} while (0)", "<stdin>: ERROR: Cannot use a declaration in a single-statement context\n")
+
+	// Test direct "eval"
+	expectPrinted(t, "if (foo) { function x() {} }", "if (foo) {\n  let x = function() {\n  };\n  var x = x;\n}\n")
+	expectPrinted(t, "if (foo) { function x() {} eval('') }", "if (foo) {\n  function x() {\n  }\n  eval(\"\");\n}\n")
+	expectPrinted(t, "if (foo) { function x() {} if (bar) { eval('') } }", "if (foo) {\n  function x() {\n  }\n  if (bar) {\n    eval(\"\");\n  }\n}\n")
+	expectPrinted(t, "if (foo) { eval(''); function x() {} }", "if (foo) {\n  function x() {\n  }\n  eval(\"\");\n}\n")
+	expectPrinted(t, "'use strict'; if (foo) { function x() {} }", "\"use strict\";\nif (foo) {\n  let x = function() {\n  };\n}\n")
+	expectPrinted(t, "'use strict'; if (foo) { function x() {} eval('') }", "\"use strict\";\nif (foo) {\n  function x() {\n  }\n  eval(\"\");\n}\n")
+	expectPrinted(t, "'use strict'; if (foo) { function x() {} if (bar) { eval('') } }", "\"use strict\";\nif (foo) {\n  function x() {\n  }\n  if (bar) {\n    eval(\"\");\n  }\n}\n")
+	expectPrinted(t, "'use strict'; if (foo) { eval(''); function x() {} }", "\"use strict\";\nif (foo) {\n  function x() {\n  }\n  eval(\"\");\n}\n")
 }
 
 func TestFunction(t *testing.T) {