JS: fix var declaration order when merging with assignment expressions,

fixes #472
tdewolff · Apr 2, 2022 · 2a0dc90 · 2a0dc90
1 parent 6442af8
commit 2a0dc90
Show file tree

Hide file tree

Showing 3 changed files with 18 additions and 12 deletions.
diff --git a/js/js_test.go b/js/js_test.go
@@ -712,6 +712,7 @@ func TestJS(t *testing.T) {
 		{`()=>({a(){b=!b}})`, `()=>({a(){b=!b}})`},                                                      // #429
 		{`var a=1;function f(){return 1}var{min,max}=Math;function g(){return 2}`, `a=1;function f(){return 1}var{min,max}=Math,a;function g(){return 2}`}, // #445
 		{`const f=x=>void console.log(x)`, `const f=x=>void console.log(x)`},                                                                               // #463
+		{`(function(){var a=b;var c=d.x,e=f.y})()`, `(function(){var a=b,c=d.x,e=f.y})()`},                                                                 // #472
 	}
 
 	m := minify.New()
@@ -780,6 +781,7 @@ func TestJSVarRenaming(t *testing.T) {
 		{`let a=0;switch(a){case 0:let b=1;case 1:let c=2}`, `let a=0;switch(a){case 0:let a=1;case 1:let b=2}`},
 		{`({a:b=1}={})=>b`, `({a=1}={})=>a`}, // #422
 		{`()=>{var a;if(x){const b=0;while(true);}}`, `()=>{if(x){const b=0;for(var a;!0;);}}`},
+		{`(e,s)=>{e=>0,s(e(s))}`, `(b,a)=>{a=>0,a(b(a))}`}, // #469
 	}
 
 	m := minify.New()

diff --git a/js/stmtlist.go b/js/stmtlist.go
@@ -198,7 +198,7 @@ func (m *jsMinifier) optimizeStmtList(list []js.IStmt, blockType blockType) []js
 							j--
 						} else if ok && (decl.TokenType == js.VarToken || decl.TokenType == js.ErrorToken) {
 							// this is the second VarDecl, so we are hoisting var declarations, which means the forInit variables are already in 'left'
-							merge := mergeVarDecls(left, decl)
+							merge := mergeVarDecls(left, decl, false)
 							if merge {
 								decl.TokenType = js.VarToken
 								forStmt.Init = left

diff --git a/js/vars.go b/js/vars.go
@@ -183,12 +183,16 @@ func addDefinition(decl *js.VarDecl, binding js.IBinding, value js.IExpr, forwar
 
 			item := decl.List[idx]
 			if v, ok := item.Binding.(*js.Var); ok && v == vbind {
-				if decl.List[idx].Default != nil {
+				if item.Default != nil {
 					return false
 				}
-				decl.List[idx].Default = value
-				decl.List = append(decl.List, decl.List[idx])
+				item.Default = value
 				decl.List = append(decl.List[:idx], decl.List[idx+1:]...)
+				if forward {
+					decl.List = append([]js.BindingElement{item}, decl.List...)
+				} else {
+					decl.List = append(decl.List, item)
+				}
 				return true
 			}
 		}
@@ -227,12 +231,12 @@ func addDefinition(decl *js.VarDecl, binding js.IBinding, value js.IExpr, forwar
 	//return true
 }
 
-func mergeVarDecls(dst, src *js.VarDecl) bool {
+func mergeVarDecls(dst, src *js.VarDecl, forward bool) bool {
 	// this is the second VarDecl, so we are hoisting var declarations, which means the forInit variables are already in 'left'
 	merge := true
 	for j := 0; j < len(src.List); j++ {
 		if src.List[j].Default != nil {
-			if addDefinition(dst, src.List[j].Binding, src.List[j].Default, false) {
+			if addDefinition(dst, src.List[j].Binding, src.List[j].Default, forward) {
 				src.List = append(src.List[:j], src.List[j+1:]...)
 				j--
 			} else {
@@ -246,20 +250,20 @@ func mergeVarDecls(dst, src *js.VarDecl) bool {
 	return merge
 }
 
-func mergeVarDeclExprStmt(decl *js.VarDecl, exprStmt *js.ExprStmt, swapped bool) bool {
+func mergeVarDeclExprStmt(decl *js.VarDecl, exprStmt *js.ExprStmt, forward bool) bool {
 	if src, ok := exprStmt.Value.(*js.VarDecl); ok {
 		// this happens when a variable declarations is converted to an expression
-		return mergeVarDecls(decl, src)
+		return mergeVarDecls(decl, src, forward)
 	} else if commaExpr, ok := exprStmt.Value.(*js.CommaExpr); ok {
 		n := 0
 		for i := 0; i < len(commaExpr.List); i++ {
 			item := commaExpr.List[i]
-			if swapped {
+			if forward {
 				item = commaExpr.List[len(commaExpr.List)-i-1]
 			}
 			if binaryExpr, ok := item.(*js.BinaryExpr); ok && binaryExpr.Op == js.EqToken {
 				if v, ok := binaryExpr.X.(*js.Var); ok && v.Decl == js.VariableDecl {
-					if addDefinition(decl, v, binaryExpr.Y, swapped) {
+					if addDefinition(decl, v, binaryExpr.Y, forward) {
 						n++
 						continue
 					}
@@ -268,15 +272,15 @@ func mergeVarDeclExprStmt(decl *js.VarDecl, exprStmt *js.ExprStmt, swapped bool)
 			break
 		}
 		merge := n == len(commaExpr.List)
-		if !swapped {
+		if !forward {
 			commaExpr.List = commaExpr.List[n:]
 		} else {
 			commaExpr.List = commaExpr.List[:len(commaExpr.List)-n]
 		}
 		return merge
 	} else if binaryExpr, ok := exprStmt.Value.(*js.BinaryExpr); ok && binaryExpr.Op == js.EqToken {
 		if v, ok := binaryExpr.X.(*js.Var); ok && v.Decl == js.VariableDecl {
-			if addDefinition(decl, v, binaryExpr.Y, swapped) {
+			if addDefinition(decl, v, binaryExpr.Y, forward) {
 				return true
 			}
 		}