Skip to content

Commit 3e9728e

Browse files
authored
fix(es/minifier): Add side effect check for test expr when compressing IfStmt (#10550)
**Description:** The minimization result of IfStmt compressing is wrong if test exprs modifies some variables that are used in the common args of cons and alt. For example: ```js // From (a = 1) ? f(a, true) : f(a, false) // To f(a, a = 1 ? true : false) ``` **Related issue:** - Closes #10473
1 parent e554381 commit 3e9728e

File tree

5 files changed

+174
-13
lines changed

5 files changed

+174
-13
lines changed

.changeset/perfect-planets-kick.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
---
2+
swc_core: patch
3+
swc_ecma_minifier: patch
4+
---
5+
6+
fix(es/minifier): add side effect check for test expr when compressing IfStmt

crates/swc_ecma_minifier/src/compress/optimize/conditionals.rs

Lines changed: 22 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -381,6 +381,12 @@ impl Optimizer<'_> {
381381

382382
match (cons, alt) {
383383
(Expr::Call(cons), Expr::Call(alt)) => {
384+
// Test expr may change the variables that cons and alt **may use** in their
385+
// common args. For example:
386+
// from (a = 1) ? f(a, true) : f(a, false)
387+
// to f(a, a = 1 ? true : false)
388+
let side_effects_in_test = test.may_have_side_effects(self.ctx.expr_ctx);
389+
384390
if self.data.contains_unresolved(test) {
385391
return None;
386392
}
@@ -411,24 +417,28 @@ impl Optimizer<'_> {
411417
&& cons.args.iter().all(|arg| arg.spread.is_none())
412418
&& alt.args.iter().all(|arg| arg.spread.is_none())
413419
{
414-
let diff_count = cons
415-
.args
416-
.iter()
417-
.zip(alt.args.iter())
418-
.filter(|(cons, alt)| !cons.eq_ignore_span(alt))
419-
.count();
420+
let mut diff_count = 0;
421+
let mut diff_idx = None;
422+
423+
for (idx, (cons, alt)) in cons.args.iter().zip(alt.args.iter()).enumerate() {
424+
if !cons.eq_ignore_span(alt) {
425+
diff_count += 1;
426+
diff_idx = Some(idx);
427+
} else {
428+
// See the comments for `side_effects_in_test`
429+
if side_effects_in_test && !cons.expr.is_pure(self.ctx.expr_ctx) {
430+
return None;
431+
}
432+
}
433+
}
420434

421435
if diff_count == 1 {
436+
let diff_idx = diff_idx.unwrap();
437+
422438
report_change!(
423439
"conditionals: Merging cons and alt as only one argument differs"
424440
);
425441
self.changed = true;
426-
let diff_idx = cons
427-
.args
428-
.iter()
429-
.zip(alt.args.iter())
430-
.position(|(cons, alt)| !cons.eq_ignore_span(alt))
431-
.unwrap();
432442

433443
let mut new_args = Vec::new();
434444

@@ -446,7 +456,6 @@ impl Optimizer<'_> {
446456
.into(),
447457
})
448458
} else {
449-
//
450459
new_args.push(arg)
451460
}
452461
}
Lines changed: 92 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,92 @@
1+
"use strict";
2+
3+
function _typeof(e) {
4+
return (_typeof =
5+
"function" == typeof Symbol && "symbol" == typeof Symbol.iterator
6+
? function (e) {
7+
return typeof e;
8+
}
9+
: function (e) {
10+
return e &&
11+
"function" == typeof Symbol &&
12+
e.constructor === Symbol &&
13+
e !== Symbol.prototype
14+
? "symbol"
15+
: typeof e;
16+
})(e);
17+
}
18+
19+
function ownKeys(t, e) {
20+
var o,
21+
r = Object.keys(t);
22+
return (
23+
Object.getOwnPropertySymbols &&
24+
((o = Object.getOwnPropertySymbols(t)),
25+
e &&
26+
(o = o.filter(function (e) {
27+
return Object.getOwnPropertyDescriptor(t, e).enumerable;
28+
})),
29+
r.push.apply(r, o)),
30+
r
31+
);
32+
}
33+
34+
function _objectSpread(t) {
35+
for (var e = 1; e < arguments.length; e++) {
36+
var o = null != arguments[e] ? arguments[e] : {};
37+
e % 2
38+
? ownKeys(Object(o), !0).forEach(function (e) {
39+
_defineProperty(t, e, o[e]);
40+
})
41+
: Object.getOwnPropertyDescriptors
42+
? Object.defineProperties(t, Object.getOwnPropertyDescriptors(o))
43+
: ownKeys(Object(o)).forEach(function (e) {
44+
Object.defineProperty(
45+
t,
46+
e,
47+
Object.getOwnPropertyDescriptor(o, e)
48+
);
49+
});
50+
}
51+
return t;
52+
}
53+
54+
function _defineProperty(e, t, o) {
55+
return (
56+
(t = _toPropertyKey(t)) in e
57+
? Object.defineProperty(e, t, {
58+
value: o,
59+
enumerable: !0,
60+
configurable: !0,
61+
writable: !0,
62+
})
63+
: (e[t] = o),
64+
e
65+
);
66+
}
67+
68+
function _toPropertyKey(e) {
69+
e = _toPrimitive(e, "string");
70+
return "symbol" == _typeof(e) ? e : e + "";
71+
}
72+
73+
function _toPrimitive(e, t) {
74+
if ("object" != _typeof(e) || !e) return e;
75+
var o = e[Symbol.toPrimitive];
76+
if (void 0 === o) return ("string" === t ? String : Number)(e);
77+
o = o.call(e, t || "default");
78+
if ("object" != _typeof(o)) return o;
79+
throw new TypeError("@@toPrimitive must return a primitive value.");
80+
}
81+
82+
var cmConfig = (cmConfig = {
83+
isToutiao: "Toutiao" === Math.random(),
84+
appid: "xxx",
85+
version: "5.8.20",
86+
}).isToutiao
87+
? _objectSpread(_objectSpread({}, cmConfig), { name: "toutiao" })
88+
: _objectSpread(_objectSpread({}, cmConfig), { name: "douyin" }),
89+
// config should equal { ...cmConfig, ...douyin }
90+
config = (exports.config = cmConfig);
91+
92+
console.log(config);
Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
"use strict";
2+
function _typeof(e) {
3+
return (_typeof = "function" == typeof Symbol && "symbol" == typeof Symbol.iterator ? function(e) {
4+
return typeof e;
5+
} : function(e) {
6+
return e && "function" == typeof Symbol && e.constructor === Symbol && e !== Symbol.prototype ? "symbol" : typeof e;
7+
})(e);
8+
}
9+
function ownKeys(t, e) {
10+
var o, r = Object.keys(t);
11+
return Object.getOwnPropertySymbols && (o = Object.getOwnPropertySymbols(t), e && (o = o.filter(function(e) {
12+
return Object.getOwnPropertyDescriptor(t, e).enumerable;
13+
})), r.push.apply(r, o)), r;
14+
}
15+
function _objectSpread(t) {
16+
for(var e = 1; e < arguments.length; e++){
17+
var o = null != arguments[e] ? arguments[e] : {};
18+
e % 2 ? ownKeys(Object(o), !0).forEach(function(e) {
19+
!function(e, t, o) {
20+
var e1;
21+
(e1 = function(e, t) {
22+
if ("object" != _typeof(e) || !e) return e;
23+
var o = e[Symbol.toPrimitive];
24+
if (void 0 === o) return ("string" === t ? String : Number)(e);
25+
if (o = o.call(e, t || "default"), "object" != _typeof(o)) return o;
26+
throw TypeError("@@toPrimitive must return a primitive value.");
27+
}(e1 = t, "string"), (t = "symbol" == _typeof(e1) ? e1 : e1 + "") in e) ? Object.defineProperty(e, t, {
28+
value: o,
29+
enumerable: !0,
30+
configurable: !0,
31+
writable: !0
32+
}) : e[t] = o;
33+
}(t, e, o[e]);
34+
}) : Object.getOwnPropertyDescriptors ? Object.defineProperties(t, Object.getOwnPropertyDescriptors(o)) : ownKeys(Object(o)).forEach(function(e) {
35+
Object.defineProperty(t, e, Object.getOwnPropertyDescriptor(o, e));
36+
});
37+
}
38+
return t;
39+
}
40+
var cmConfig = (cmConfig = {
41+
isToutiao: "Toutiao" === Math.random(),
42+
appid: "xxx",
43+
version: "5.8.20"
44+
}).isToutiao ? _objectSpread(_objectSpread({}, cmConfig), {
45+
name: "toutiao"
46+
}) : _objectSpread(_objectSpread({}, cmConfig), {
47+
name: "douyin"
48+
});
49+
console.log(exports.config = cmConfig);

crates/swc_ecma_utils/src/lib.rs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -721,6 +721,11 @@ pub trait ExprExt {
721721
is_one_of_global_ref_to(self.as_expr(), ctx, ids)
722722
}
723723

724+
#[inline(always)]
725+
fn is_pure(&self, ctx: ExprCtx) -> bool {
726+
self.as_pure_bool(ctx).is_known()
727+
}
728+
724729
/// Get bool value of `self` if it does not have any side effects.
725730
#[inline(always)]
726731
fn as_pure_bool(&self, ctx: ExprCtx) -> BoolValue {

0 commit comments

Comments
 (0)