avoid var declaration overwritten

- fix #2080 regression introduced in 4fa3d7a
evanw · Mar 13, 2022 · 47802b7 · 47802b7
1 parent 9fe4a61
commit 47802b7
Show file tree

Hide file tree

Showing 5 changed files with 41 additions and 22 deletions.
diff --git a/internal/bundler/snapshots/snapshots_default.txt b/internal/bundler/snapshots/snapshots_default.txt
@@ -36,7 +36,7 @@ TestArgumentsSpecialCaseNoBundle
 ---------- /out.js ----------
 (() => {
   var r;
-  function t(n = arguments) {
+  function a(n = arguments) {
     return arguments;
   }
   (function(n = arguments) {
@@ -45,7 +45,7 @@ TestArgumentsSpecialCaseNoBundle
   ({ foo(n = arguments) {
     return arguments;
   } });
-  class u {
+  class t {
     foo(e = arguments) {
       return arguments;
     }
@@ -55,7 +55,7 @@ TestArgumentsSpecialCaseNoBundle
       return arguments;
     }
   });
-  function t(n = arguments) {
+  function u(n = arguments) {
     var arguments;
     return arguments;
   }

diff --git a/internal/bundler/snapshots/snapshots_loader.txt b/internal/bundler/snapshots/snapshots_loader.txt
@@ -874,9 +874,9 @@ var i=r((t,e)=>{e.exports=123});var s=i();console.log(s,"no identifier in this f
 ================================================================================
 TestNamedFunctionExpressionArgumentCollision
 ---------- /out/entry.js ----------
-let x = function(foo) {
-  var foo;
-  return foo;
+let x = function(foo2) {
+  var foo2;
+  return foo2;
 };
 
 ================================================================================

diff --git a/internal/js_ast/js_ast.go b/internal/js_ast/js_ast.go
@@ -1482,10 +1482,11 @@ type Scope struct {
 	// This will be non-nil if this is a TypeScript "namespace" or "enum"
 	TSNamespace *TSNamespaceScope
 
-	Parent    *Scope
-	Children  []*Scope
-	Members   map[string]ScopeMember
-	Generated []Ref
+	Parent     *Scope
+	Children   []*Scope
+	Members    map[string]ScopeMember
+	HoistFnRef map[string]*Ref
+	Generated  []Ref
 
 	// The location of the "use strict" directive for ExplicitStrictMode
 	UseStrictLoc logger.Loc

diff --git a/internal/js_parser/js_parser.go b/internal/js_parser/js_parser.go
@@ -910,10 +910,11 @@ func (p *parser) selectLocalKind(kind js_ast.LocalKind) js_ast.LocalKind {
 func (p *parser) pushScopeForParsePass(kind js_ast.ScopeKind, loc logger.Loc) int {
 	parent := p.currentScope
 	scope := &js_ast.Scope{
-		Kind:    kind,
-		Parent:  parent,
-		Members: make(map[string]js_ast.ScopeMember),
-		Label:   js_ast.LocRef{Ref: js_ast.InvalidRef},
+		Kind:       kind,
+		Parent:     parent,
+		Members:    make(map[string]js_ast.ScopeMember),
+		HoistFnRef: make(map[string]*js_ast.Ref),
+		Label:      js_ast.LocRef{Ref: js_ast.InvalidRef},
 	}
 	if parent != nil {
 		parent.Children = append(parent.Children, scope)
@@ -1107,6 +1108,7 @@ const (
 	mergeForbidden = iota
 	mergeReplaceWithNew
 	mergeOverwriteWithNew
+	mergeHoistFunction
 	mergeKeepExisting
 	mergeBecomePrivateGetSetPair
 	mergeBecomePrivateStaticGetSetPair
@@ -1147,14 +1149,12 @@ func (p *parser) canMergeSymbols(scope *js_ast.Scope, existing js_ast.SymbolKind
 		}
 	}
 
-	// "var foo; var foo;"
-	// "var foo; function foo() {}"
-	// "function foo() {} var foo;"
+	// only top level function can be overwritten
 	// "function *foo() {} function *foo() {}" but not "{ function *foo() {} function *foo() {} }"
 	if new.IsHoistedOrFunction() && existing.IsHoistedOrFunction() &&
 		(scope.Kind == js_ast.ScopeEntry || scope.Kind == js_ast.ScopeFunctionBody ||
 			(new.IsHoisted() && existing.IsHoisted())) {
-		return mergeReplaceWithNew
+		return mergeHoistFunction
 	}
 
 	// "get #foo() {} set #foo() {}"
@@ -1219,9 +1219,25 @@ func (p *parser) declareSymbol(kind js_ast.SymbolKind, loc logger.Loc, name stri
 		case mergeReplaceWithNew:
 			symbol.Link = ref
 
-			// If these are both functions, remove the overwritten declaration
-			if p.options.minifySyntax && kind.IsFunction() && symbol.Kind.IsFunction() {
-				symbol.Flags |= js_ast.RemoveOverwrittenFunctionDeclaration
+		case mergeHoistFunction:
+			if symbol.Kind.IsFunction() {
+				p.currentScope.HoistFnRef[name] = &existing.Ref
+			}
+
+			if kind.IsFunction() {
+				if p.options.minifySyntax {
+					fnRef := p.currentScope.HoistFnRef[name]
+					if fnRef != nil {
+						fnSymbol := &p.symbols[fnRef.InnerIndex]
+						fnSymbol.Flags |= js_ast.RemoveOverwrittenFunctionDeclaration
+					}
+				}
+
+				p.currentScope.HoistFnRef[name] = &ref
+			}
+
+			if !symbol.Kind.IsFunction() && !kind.IsFunction() {
+				ref = existing.Ref
 			}
 
 		case mergeBecomePrivateGetSetPair:

diff --git a/internal/js_parser/js_parser_test.go b/internal/js_parser/js_parser_test.go
@@ -1357,8 +1357,10 @@ func TestFunction(t *testing.T) {
 	expectPrintedMangle(t, "function f() {} var f", "function f() {\n}\nvar f;\n")
 	expectPrintedMangle(t, "var f; function f() { x() } function f() { y() }", "var f;\nfunction f() {\n  y();\n}\n")
 	expectPrintedMangle(t, "function f() { x() } function f() { y() } var f", "function f() {\n  y();\n}\nvar f;\n")
-	expectPrintedMangle(t, "function f() { x() } var f; function f() { y() }", "function f() {\n  x();\n}\nvar f;\nfunction f() {\n  y();\n}\n")
+	expectPrintedMangle(t, "function f() { x() } var f; function f() { y() }", "var f;\nfunction f() {\n  y();\n}\n")
 	expectPrintedMangle(t, "export function f() { x() } function f() { y() }", "export function f() {\n  x();\n}\nfunction f() {\n  y();\n}\n")
+
+	expectPrintedMangle(t, "var x = x || {}; console.log(x); var x = x || {};", "var x = x || {};\nconsole.log(x);\nvar x = x || {};\n")
 }
 
 func TestClass(t *testing.T) {