fix #2361: keeps inlined const for direct eval

CHANGELOG.md

@@ -2,6 +2,21 @@
 
 ## Unreleased
 
+* Keep inlined constants when direct `eval` is present ([#2361](https://github.com/evanw/esbuild/issues/2361))
+
+    Version 0.14.19 of esbuild added inlining of certain `const` variables during minification, which replaces all references to the variable with the initializer and then removes the variable declaration. However, this could generate incorrect code when direct `eval` is present because the direct `eval` could reference the constant by name. This release fixes the problem by preserving the `const` variable declaration in this case:
+
+    ```js
+    // Original code
+    console.log((() => { const x = 123; return x + eval('x') }))
+
+    // Old output (with --minify)
+    console.log(()=>123+eval("x"));
+
+    // New output (with --minify)
+    console.log(()=>{const x=123;return 123+eval("x")});
+    ```
+
 * Fix an incorrect error in TypeScript when targeting ES5 ([#2375](https://github.com/evanw/esbuild/issues/2375))
 
     Previously when compiling TypeScript code to ES5, esbuild could incorrectly consider the following syntax forms as a transformation error:

internal/bundler/bundler_dce_test.go

@@ -3014,6 +3014,58 @@ const-update.js: NOTE: The symbol "x" was declared a constant here:
 	})
 }
 
+func TestConstValueInliningDirectEval(t *testing.T) {
+	dce_suite.expectBundled(t, bundled{
+		files: map[string]string{
+			"/top-level-no-eval.js": `
+				const x = 1
+				console.log(x, evil('x'))
+			`,
+			"/top-level-eval.js": `
+				const x = 1
+				console.log(x, eval('x'))
+			`,
+			"/nested-no-eval.js": `
+				(() => {
+					const x = 1
+					console.log(x, evil('x'))
+				})()
+			`,
+			"/nested-eval.js": `
+				(() => {
+					const x = 1
+					console.log(x, eval('x'))
+				})()
+			`,
+			"/ts-namespace-no-eval.ts": `
+				namespace y {
+					export const x = 1
+					console.log(x, evil('x'))
+				}
+			`,
+			"/ts-namespace-eval.ts": `
+				namespace z {
+					export const x = 1
+					console.log(x, eval('x'))
+				}
+			`,
+		},
+		entryPaths: []string{
+			"/top-level-no-eval.js",
+			"/top-level-eval.js",
+			"/nested-no-eval.js",
+			"/nested-eval.js",
+			"/ts-namespace-no-eval.ts",
+			"/ts-namespace-eval.ts",
+		},
+		options: config.Options{
+			Mode:         config.ModePassThrough,
+			AbsOutputDir: "/out",
+			MinifySyntax: true,
+		},
+	})
+}
+
 func TestCrossModuleConstantFolding(t *testing.T) {
 	dce_suite.expectBundled(t, bundled{
 		files: map[string]string{

internal/bundler/snapshots/snapshots_dce.txt

@@ -96,6 +96,33 @@ function bar() {
 // non-circular-export-entry.js
 console.log(123, bar());
 
+================================================================================
+TestConstValueInliningDirectEval
+---------- /out/top-level-no-eval.js ----------
+const x = 1;
+console.log(1, evil("x"));
+
+---------- /out/top-level-eval.js ----------
+const x = 1;
+console.log(1, eval("x"));
+
+---------- /out/nested-no-eval.js ----------
+console.log(1, evil("x"));
+
+---------- /out/nested-eval.js ----------
+(() => {
+  const x = 1;
+  console.log(1, eval("x"));
+})();
+
+---------- /out/ts-namespace-no-eval.js ----------
+var y;
+((y2) => (y2.x = 1, console.log(1, evil("x"))))(y ||= {});
+
+---------- /out/ts-namespace-eval.js ----------
+var z;
+((z) => (z.x = 1, console.log(1, eval("x"))))(z ||= {});
+
 ================================================================================
 TestConstValueInliningNoBundle
 ---------- /out/top-level.js ----------

internal/js_ast/js_ast.go

@@ -486,6 +486,8 @@ var ESuperShared = &ESuper{}
 var ENullShared = &ENull{}
 var EUndefinedShared = &EUndefined{}
 var EThisShared = &EThis{}
+var SEmptyShared = &SEmpty{}
+var SDebuggerShared = &SDebugger{}
 
 type ENew struct {
 	Target        Expr

internal/js_parser/js_parser.go

@@ -5825,7 +5825,7 @@ func (p *parser) parseStmt(opts parseStmtOpts) js_ast.Stmt {
 	switch p.lexer.Token {
 	case js_lexer.TSemicolon:
 		p.lexer.Next()
-		return js_ast.Stmt{Loc: loc, Data: &js_ast.SEmpty{}}
+		return js_ast.Stmt{Loc: loc, Data: js_ast.SEmptyShared}
 
 	case js_lexer.TExport:
 		previousExportKeyword := p.esmExportKeyword
@@ -6805,7 +6805,7 @@ func (p *parser) parseStmt(opts parseStmtOpts) js_ast.Stmt {
 	case js_lexer.TDebugger:
 		p.lexer.Next()
 		p.lexer.ExpectOrInsertSemicolon()
-		return js_ast.Stmt{Loc: loc, Data: &js_ast.SDebugger{}}
+		return js_ast.Stmt{Loc: loc, Data: js_ast.SDebuggerShared}
 
 	case js_lexer.TOpenBrace:
 		p.pushScopeForParsePass(js_ast.ScopeBlock, loc)
@@ -7606,6 +7606,41 @@ func isDirectiveSupported(s *js_ast.SDirective) bool {
 }
 
 func (p *parser) mangleStmts(stmts []js_ast.Stmt, kind stmtsKind) []js_ast.Stmt {
+	// Remove inlined constants now that we know whether any of these statements
+	// contained a direct eval() or not. This can't be done earlier when we
+	// encounter the constant because we haven't encountered the eval() yet.
+	// Inlined constants are not removed if they are in a top-level scope or
+	// if they are exported (which could be in a nested TypeScript namespace).
+	if p.currentScope.Parent != nil && !p.currentScope.ContainsDirectEval {
+		for i, stmt := range stmts {
+			switch s := stmt.Data.(type) {
+			case *js_ast.SEmpty, *js_ast.SComment, *js_ast.SDirective, *js_ast.SDebugger, *js_ast.STypeScript:
+				continue
+
+			case *js_ast.SLocal:
+				if !s.IsExport {
+					end := 0
+					for _, d := range s.Decls {
+						if id, ok := d.Binding.Data.(*js_ast.BIdentifier); ok {
+							if _, ok := p.constValues[id.Ref]; ok {
+								continue
+							}
+						}
+						s.Decls[end] = d
+						end++
+					}
+					if end == 0 {
+						stmts[i].Data = js_ast.SEmptyShared
+					} else {
+						s.Decls = s.Decls[:end]
+					}
+				}
+				continue
+			}
+			break
+		}
+	}
+
 	// Merge adjacent statements during mangling
 	result := make([]js_ast.Stmt, 0, len(stmts))
 	isControlFlowDead := false
@@ -8402,7 +8437,7 @@ func (p *parser) visitSingleStmt(stmt js_ast.Stmt, kind stmtsKind) js_ast.Stmt {
 // One statement could potentially expand to several statements
 func stmtsToSingleStmt(loc logger.Loc, stmts []js_ast.Stmt) js_ast.Stmt {
 	if len(stmts) == 0 {
-		return js_ast.Stmt{Loc: loc, Data: &js_ast.SEmpty{}}
+		return js_ast.Stmt{Loc: loc, Data: js_ast.SEmptyShared}
 	}
 	if len(stmts) == 1 {
 		// "let" and "const" must be put in a block when in a single-statement context
@@ -8547,7 +8582,7 @@ func dropFirstStatement(body js_ast.Stmt, replaceOrNil js_ast.Stmt) js_ast.Stmt
 	if replaceOrNil.Data != nil {
 		return replaceOrNil
 	}
-	return js_ast.Stmt{Loc: body.Loc, Data: &js_ast.SEmpty{}}
+	return js_ast.Stmt{Loc: body.Loc, Data: js_ast.SEmptyShared}
 }
 
 func mangleFor(s *js_ast.SFor) {
@@ -9252,9 +9287,8 @@ func (p *parser) visitAndAppendStmt(stmts []js_ast.Stmt, stmt js_ast.Stmt) []js_
 		// Local statements do not end the const local prefix
 		p.currentScope.IsAfterConstLocalPrefix = wasAfterAfterConstLocalPrefix
 
-		end := 0
 		for i := range s.Decls {
-			d := s.Decls[i]
+			d := &s.Decls[i]
 			p.visitBinding(d.Binding, bindingOpts{})
 
 			// Visit the initializer
@@ -9306,13 +9340,6 @@ func (p *parser) visitAndAppendStmt(stmts []js_ast.Stmt, stmt js_ast.Stmt) []js_
 								p.constValues = make(map[js_ast.Ref]js_ast.ConstValue)
 							}
 							p.constValues[id.Ref] = value
-
-							// Only keep this declaration if it's top-level or exported (which
-							// could be in a nested TypeScript namespace), otherwise erase it
-							if p.currentScope.Parent == nil || s.IsExport {
-								s.Decls[end] = d
-								end++
-							}
 							continue
 						}
 					}
@@ -9325,20 +9352,8 @@ func (p *parser) visitAndAppendStmt(stmts []js_ast.Stmt, stmt js_ast.Stmt) []js_
 					p.currentScope.IsAfterConstLocalPrefix = true
 				}
 			}
-
-			// Keep the declaration if we didn't continue the loop above
-			s.Decls[end] = d
-			end++
-		}
-
-		// Remove this declaration entirely if all symbols are inlined constants
-		if end == 0 {
-			return stmts
 		}
 
-		// Trim the removed declarations
-		s.Decls = s.Decls[:end]
-
 		// Handle being exported inside a namespace
 		if s.IsExport && p.enclosingNamespaceArgRef != nil {
 			wrapIdentifier := func(loc logger.Loc, ref js_ast.Ref) js_ast.Expr {
@@ -9444,7 +9459,7 @@ func (p *parser) visitAndAppendStmt(stmts []js_ast.Stmt, stmt js_ast.Stmt) []js_
 				stmt = s.Stmts[0]
 			} else if len(s.Stmts) == 0 {
 				// Trim empty blocks
-				stmt = js_ast.Stmt{Loc: stmt.Loc, Data: &js_ast.SEmpty{}}
+				stmt = js_ast.Stmt{Loc: stmt.Loc, Data: js_ast.SEmptyShared}
 			}
 		}