Skip to content

Commit 20878ce

Browse files
evanwevanw
committed
fix #2445: fixes for spread arguments and --minify
1 parent b2b5a60 commit 20878ce

File tree

8 files changed

+274
-14
lines changed

8 files changed

+274
-14
lines changed

‎CHANGELOG.md

+24
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,29 @@
11
# Changelog
22

3+
## Unreleased
4+
5+
* Fix optimizations for calls containing spread arguments ([#2445](https://github.com/evanw/esbuild/issues/2445))
6+
7+
This release fixes the handling of spread arguments in the optimization of `/* @__PURE__ */` comments, empty functions, and identity functions:
8+
9+
```js
10+
// Original code
11+
function empty() {}
12+
function identity(x) { return x }
13+
/* @__PURE__ */ a(...x)
14+
/* @__PURE__ */ new b(...x)
15+
empty(...x)
16+
identity(...x)
17+
18+
// Old output (with --minify --tree-shaking=true)
19+
...x;...x;...x;...x;
20+
21+
// New output (with --minify --tree-shaking=true)
22+
function identity(n){return n}[...x];[...x];[...x];identity(...x);
23+
```
24+
25+
Previously esbuild assumed arguments with side effects could be directly inlined. This is almost always true except for spread arguments, which are not syntactically valid on their own and which have the side effect of causing iteration, which might have further side effects. Now esbuild will wrap these elements in an unused array so that they are syntactically valid and so that the iteration side effects are preserved.
26+
327
## 0.14.53
428

529
This release fixes a minor issue with the previous release: I had to rename the package `esbuild-linux-loong64` to `@esbuild/linux-loong64` in the contributed PR because someone registered the package name before I could claim it, and I missed a spot. Hopefully everything is working after this release. I plan to change all platform-specific package names to use the `@esbuild/` scope at some point to avoid this problem in the future.

‎internal/bundler/bundler_dce_test.go

+142
Original file line numberDiff line numberDiff line change
@@ -3271,3 +3271,145 @@ func TestMultipleDeclarationTreeShakingMinifySyntax(t *testing.T) {
32713271
},
32723272
})
32733273
}
3274+
3275+
// Pure call removal should still run iterators, which can have side effects
3276+
func TestPureCallsWithSpread(t *testing.T) {
3277+
dce_suite.expectBundled(t, bundled{
3278+
files: map[string]string{
3279+
"/entry.js": `
3280+
/* @__PURE__ */ foo(...args);
3281+
/* @__PURE__ */ new foo(...args);
3282+
`,
3283+
},
3284+
entryPaths: []string{"/entry.js"},
3285+
options: config.Options{
3286+
Mode: config.ModeBundle,
3287+
AbsOutputFile: "/out.js",
3288+
MinifySyntax: true,
3289+
},
3290+
})
3291+
}
3292+
3293+
func TestTopLevelFunctionInliningWithSpread(t *testing.T) {
3294+
dce_suite.expectBundled(t, bundled{
3295+
files: map[string]string{
3296+
"/entry.js": `
3297+
function empty1() {}
3298+
function empty2() {}
3299+
function empty3() {}
3300+
3301+
function identity1(x) { return x }
3302+
function identity2(x) { return x }
3303+
function identity3(x) { return x }
3304+
3305+
empty1()
3306+
empty2(args)
3307+
empty3(...args)
3308+
3309+
identity1()
3310+
identity2(args)
3311+
identity3(...args)
3312+
`,
3313+
3314+
"/inner.js": `
3315+
export function empty1() {}
3316+
export function empty2() {}
3317+
export function empty3() {}
3318+
3319+
export function identity1(x) { return x }
3320+
export function identity2(x) { return x }
3321+
export function identity3(x) { return x }
3322+
`,
3323+
3324+
"/entry-outer.js": `
3325+
import {
3326+
empty1,
3327+
empty2,
3328+
empty3,
3329+
3330+
identity1,
3331+
identity2,
3332+
identity3,
3333+
} from './inner.js'
3334+
3335+
empty1()
3336+
empty2(args)
3337+
empty3(...args)
3338+
3339+
identity1()
3340+
identity2(args)
3341+
identity3(...args)
3342+
`,
3343+
},
3344+
entryPaths: []string{"/entry.js", "/entry-outer.js"},
3345+
options: config.Options{
3346+
Mode: config.ModeBundle,
3347+
AbsOutputDir: "/out",
3348+
MinifySyntax: true,
3349+
},
3350+
})
3351+
}
3352+
3353+
func TestNestedFunctionInliningWithSpread(t *testing.T) {
3354+
dce_suite.expectBundled(t, bundled{
3355+
files: map[string]string{
3356+
"/entry.js": `
3357+
function empty1() {}
3358+
function empty2() {}
3359+
function empty3() {}
3360+
3361+
function identity1(x) { return x }
3362+
function identity2(x) { return x }
3363+
function identity3(x) { return x }
3364+
3365+
check(
3366+
empty1(),
3367+
empty2(args),
3368+
empty3(...args),
3369+
3370+
identity1(),
3371+
identity2(args),
3372+
identity3(...args),
3373+
)
3374+
`,
3375+
3376+
"/inner.js": `
3377+
export function empty1() {}
3378+
export function empty2() {}
3379+
export function empty3() {}
3380+
3381+
export function identity1(x) { return x }
3382+
export function identity2(x) { return x }
3383+
export function identity3(x) { return x }
3384+
`,
3385+
3386+
"/entry-outer.js": `
3387+
import {
3388+
empty1,
3389+
empty2,
3390+
empty3,
3391+
3392+
identity1,
3393+
identity2,
3394+
identity3,
3395+
} from './inner.js'
3396+
3397+
check(
3398+
empty1(),
3399+
empty2(args),
3400+
empty3(...args),
3401+
3402+
identity1(),
3403+
identity2(args),
3404+
identity3(...args),
3405+
)
3406+
`,
3407+
},
3408+
entryPaths: []string{"/entry.js", "/entry-outer.js"},
3409+
options: config.Options{
3410+
Mode: config.ModeBundle,
3411+
AbsOutputDir: "/out",
3412+
MinifySyntax: true,
3413+
},
3414+
})
3415+
}

‎internal/bundler/linker.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -1566,8 +1566,8 @@ func (c *linkerContext) scanImportsAndExports() {
15661566
// Every call will be inlined
15671567
continue
15681568
} else if (flags & (js_ast.IsIdentityFunction | js_ast.CouldPotentiallyBeMutated)) == js_ast.IsIdentityFunction {
1569-
// Every single-argument call will be inlined
1570-
callUse.CallCountEstimate -= callUse.SingleArgCallCountEstimate
1569+
// Every single-argument call will be inlined as long as it's not a spread
1570+
callUse.CallCountEstimate -= callUse.SingleArgNonSpreadCallCountEstimate
15711571
if callUse.CallCountEstimate == 0 {
15721572
continue
15731573
}

‎internal/bundler/snapshots/snapshots_dce.txt

+77
Original file line numberDiff line numberDiff line change
@@ -1043,6 +1043,44 @@ function x() {
10431043
return 3;
10441044
}
10451045

1046+
================================================================================
1047+
TestNestedFunctionInliningWithSpread
1048+
---------- /out/entry.js ----------
1049+
// entry.js
1050+
function identity1(x) {
1051+
return x;
1052+
}
1053+
function identity3(x) {
1054+
return x;
1055+
}
1056+
check(
1057+
void 0,
1058+
(args, void 0),
1059+
([...args], void 0),
1060+
identity1(),
1061+
args,
1062+
identity3(...args)
1063+
);
1064+
1065+
---------- /out/entry-outer.js ----------
1066+
// inner.js
1067+
function identity1(x) {
1068+
return x;
1069+
}
1070+
function identity3(x) {
1071+
return x;
1072+
}
1073+
1074+
// entry-outer.js
1075+
check(
1076+
void 0,
1077+
(args, void 0),
1078+
([...args], void 0),
1079+
identity1(),
1080+
args,
1081+
identity3(...args)
1082+
);
1083+
10461084
================================================================================
10471085
TestPackageJsonSideEffectsArrayGlob
10481086
---------- /out.js ----------
@@ -1455,6 +1493,13 @@ console.log("hello");
14551493
// Users/user/project/src/entry.js
14561494
console.log("unused import");
14571495

1496+
================================================================================
1497+
TestPureCallsWithSpread
1498+
---------- /out.js ----------
1499+
// entry.js
1500+
[...args];
1501+
[...args];
1502+
14581503
================================================================================
14591504
TestRemoveTrailingReturn
14601505
---------- /out.js ----------
@@ -1545,6 +1590,38 @@ TestTextLoaderRemoveUnused
15451590
// entry.js
15461591
console.log("unused import");
15471592

1593+
================================================================================
1594+
TestTopLevelFunctionInliningWithSpread
1595+
---------- /out/entry.js ----------
1596+
// entry.js
1597+
function identity1(x) {
1598+
return x;
1599+
}
1600+
function identity3(x) {
1601+
return x;
1602+
}
1603+
args;
1604+
[...args];
1605+
identity1();
1606+
args;
1607+
identity3(...args);
1608+
1609+
---------- /out/entry-outer.js ----------
1610+
// inner.js
1611+
function identity1(x) {
1612+
return x;
1613+
}
1614+
function identity3(x) {
1615+
return x;
1616+
}
1617+
1618+
// entry-outer.js
1619+
args;
1620+
[...args];
1621+
identity1();
1622+
args;
1623+
identity3(...args);
1624+
15481625
================================================================================
15491626
TestTreeShakingBinaryOperators
15501627
---------- /out.js ----------

‎internal/js_ast/js_ast.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -2100,8 +2100,8 @@ type SymbolUse struct {
21002100
}
21012101

21022102
type SymbolCallUse struct {
2103-
CallCountEstimate uint32
2104-
SingleArgCallCountEstimate uint32
2103+
CallCountEstimate uint32
2104+
SingleArgNonSpreadCallCountEstimate uint32
21052105
}
21062106

21072107
// Returns the canonical ref that represents the ref for the provided symbol.

‎internal/js_ast/js_ast_helpers.go

+6
Original file line numberDiff line numberDiff line change
@@ -676,6 +676,9 @@ func SimplifyUnusedExpr(expr Expr, unsupportedFeatures compat.JSFeature, isUnbou
676676
if e.CanBeUnwrappedIfUnused {
677677
var result Expr
678678
for _, arg := range e.Args {
679+
if _, ok := arg.Data.(*ESpread); ok {
680+
arg.Data = &EArray{Items: []Expr{arg}, IsSingleLine: true}
681+
}
679682
result = JoinWithComma(result, SimplifyUnusedExpr(arg, unsupportedFeatures, isUnbound))
680683
}
681684
return result
@@ -732,6 +735,9 @@ func SimplifyUnusedExpr(expr Expr, unsupportedFeatures compat.JSFeature, isUnbou
732735
if e.CanBeUnwrappedIfUnused {
733736
var result Expr
734737
for _, arg := range e.Args {
738+
if _, ok := arg.Data.(*ESpread); ok {
739+
arg.Data = &EArray{Items: []Expr{arg}, IsSingleLine: true}
740+
}
735741
result = JoinWithComma(result, SimplifyUnusedExpr(arg, unsupportedFeatures, isUnbound))
736742
}
737743
return result

‎internal/js_parser/js_parser.go

+5-5
Original file line numberDiff line numberDiff line change
@@ -14048,7 +14048,7 @@ func (p *parser) visitExprInOut(expr js_ast.Expr, in exprIn) (js_ast.Expr, exprO
1404814048
case *js_ast.EImportIdentifier:
1404914049
// If this function is inlined, allow it to be tree-shaken
1405014050
if p.options.minifySyntax && !p.isControlFlowDead {
14051-
p.convertSymbolUseToCall(t.Ref, len(e.Args) == 1)
14051+
p.convertSymbolUseToCall(t.Ref, len(e.Args) == 1 && !hasSpread)
1405214052
}
1405314053

1405414054
case *js_ast.EIdentifier:
@@ -14095,7 +14095,7 @@ func (p *parser) visitExprInOut(expr js_ast.Expr, in exprIn) (js_ast.Expr, exprO
1409514095

1409614096
// If this function is inlined, allow it to be tree-shaken
1409714097
if p.options.minifySyntax && !p.isControlFlowDead {
14098-
p.convertSymbolUseToCall(t.Ref, len(e.Args) == 1)
14098+
p.convertSymbolUseToCall(t.Ref, len(e.Args) == 1 && !hasSpread)
1409914099
}
1410014100

1410114101
case *js_ast.EDot:
@@ -14385,7 +14385,7 @@ func (p *parser) visitExprInOut(expr js_ast.Expr, in exprIn) (js_ast.Expr, exprO
1438514385
return expr, exprOut{}
1438614386
}
1438714387

14388-
func (p *parser) convertSymbolUseToCall(ref js_ast.Ref, isSingleArgCall bool) {
14388+
func (p *parser) convertSymbolUseToCall(ref js_ast.Ref, isSingleNonSpreadArgCall bool) {
1438914389
// Remove the normal symbol use
1439014390
use := p.symbolUses[ref]
1439114391
use.CountEstimate--
@@ -14401,8 +14401,8 @@ func (p *parser) convertSymbolUseToCall(ref js_ast.Ref, isSingleArgCall bool) {
1440114401
}
1440214402
callUse := p.symbolCallUses[ref]
1440314403
callUse.CallCountEstimate++
14404-
if isSingleArgCall {
14405-
callUse.SingleArgCallCountEstimate++
14404+
if isSingleNonSpreadArgCall {
14405+
callUse.SingleArgNonSpreadCallCountEstimate++
1440614406
}
1440714407
p.symbolCallUses[ref] = callUse
1440814408
}

0 commit comments

Comments
 (0)