Skip to content

Commit 0e804aa

Browse files
committed
fix(minifier): keep variables that are modified by combined assignments made by minification (#13267)
```js (function() { let v; window.foo = function() { return v ?? (v = bar()); } })() ``` was compressed to ```js (function() { window.foo = function() { return bar(); } })() ``` . But this changes the semantics because `bar()` will be executed every time `window.foo()` is called after the minification. This was caused by not updating the semantics information when compressing `v ?? (v = bar())` into `v ??= bar()` (and other similar ones). This PR fixes that by updating the semantics information when those compressions are applied.
1 parent 6003285 commit 0e804aa

File tree

3 files changed

+99
-0
lines changed

3 files changed

+99
-0
lines changed

crates/oxc_minifier/src/peephole/minimize_conditions.rs

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ use oxc_ecmascript::{
44
constant_evaluation::{ConstantEvaluation, ConstantValue, DetermineValueType},
55
side_effects::MayHaveSideEffects,
66
};
7+
use oxc_semantic::ReferenceFlags;
78
use oxc_span::GetSpan;
89
use oxc_syntax::es_target::ESTarget;
910

@@ -200,6 +201,9 @@ impl<'a> PeepholeOptimizations {
200201
return;
201202
}
202203

204+
let reference = ctx.scoping_mut().get_reference_mut(write_id_ref.reference_id());
205+
reference.flags_mut().insert(ReferenceFlags::Read);
206+
203207
let new_op = logical_expr.operator.to_assignment_operator();
204208
expr.operator = new_op;
205209
expr.right = logical_expr.right.take_in(ctx.ast);
@@ -220,6 +224,9 @@ impl<'a> PeepholeOptimizations {
220224
{
221225
return;
222226
}
227+
228+
Self::mark_assignment_target_as_read(&expr.left, ctx);
229+
223230
expr.operator = new_op;
224231
expr.right = binary_expr.right.take_in(ctx.ast);
225232
ctx.state.changed = true;

crates/oxc_minifier/src/peephole/minimize_logical_expression.rs

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
use oxc_allocator::TakeIn;
22
use oxc_ast::ast::*;
3+
use oxc_semantic::ReferenceFlags;
34
use oxc_span::{ContentEq, GetSpan};
45
use oxc_syntax::es_target::ESTarget;
56

@@ -236,6 +237,8 @@ impl<'a> PeepholeOptimizations {
236237
unreachable!()
237238
};
238239

240+
Self::mark_assignment_target_as_read(&assignment_expr.left, ctx);
241+
239242
let assign_value = assignment_expr.right.take_in(ctx.ast);
240243
sequence_expr.expressions.push(assign_value);
241244
*expr = ctx.ast.expression_assignment(
@@ -259,6 +262,9 @@ impl<'a> PeepholeOptimizations {
259262
{
260263
return;
261264
}
265+
266+
Self::mark_assignment_target_as_read(&assignment_expr.left, ctx);
267+
262268
let span = e.span;
263269
let Expression::AssignmentExpression(assignment_expr) = &mut e.right else {
264270
return;
@@ -268,4 +274,15 @@ impl<'a> PeepholeOptimizations {
268274
*expr = e.right.take_in(ctx.ast);
269275
ctx.state.changed = true;
270276
}
277+
278+
/// Marks the AssignmentTargetIdentifier of assignment expressions as ReferenceFlags::Read
279+
///
280+
/// When creating AssignmentTargetIdentifier from normal expressions, the identifier only has ReferenceFlags::Write.
281+
/// But assignment expressions changes the value, so we should add ReferenceFlags::Read.
282+
pub fn mark_assignment_target_as_read(assign_target: &AssignmentTarget, ctx: &mut Ctx<'a, '_>) {
283+
if let AssignmentTarget::AssignmentTargetIdentifier(id) = assign_target {
284+
let reference = ctx.scoping_mut().get_reference_mut(id.reference_id());
285+
reference.flags_mut().insert(ReferenceFlags::Read);
286+
}
287+
}
271288
}

crates/oxc_minifier/tests/peephole/oxc.rs

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,13 @@
1+
use oxc_minifier::CompressOptions;
2+
3+
use super::super::test as test_options;
14
/// Oxc Integration Tests
25
use super::{test, test_same};
36

7+
fn test_unused(source_text: &str, expected: &str) {
8+
test_options(source_text, expected, CompressOptions::default());
9+
}
10+
411
#[test]
512
fn integration() {
613
test(
@@ -104,3 +111,71 @@ fn eval() {
104111
test_same("eval?.(x, y)");
105112
test_same("eval?.(x,y)");
106113
}
114+
115+
#[test]
116+
fn unused() {
117+
test_unused(
118+
"(function() {
119+
let v;
120+
window.foo = function() {
121+
return v ?? (v = bar());
122+
}
123+
})()
124+
",
125+
"(function() {
126+
let v;
127+
window.foo = function() {
128+
return v ??= bar();
129+
}
130+
})()
131+
",
132+
);
133+
test_unused(
134+
"(function() {
135+
let v;
136+
window.foo = function() {
137+
return v ?? (console.log(), v = bar());
138+
}
139+
})()
140+
",
141+
"(function() {
142+
let v;
143+
window.foo = function() {
144+
return v ??= (console.log(), bar());
145+
}
146+
})()
147+
",
148+
);
149+
test_unused(
150+
"(function() {
151+
let v;
152+
window.foo = function() {
153+
return v = v || bar();
154+
}
155+
})()
156+
",
157+
"(function() {
158+
let v;
159+
window.foo = function() {
160+
return v ||= bar();
161+
}
162+
})()
163+
",
164+
);
165+
test_unused(
166+
"(function() {
167+
let v;
168+
window.foo = function() {
169+
return v = v + bar();
170+
}
171+
})()
172+
",
173+
"(function() {
174+
let v;
175+
window.foo = function() {
176+
return v += bar();
177+
}
178+
})()
179+
",
180+
);
181+
}

0 commit comments

Comments
 (0)