fix(minifier): mark LHS of ??= as read when converting from == null &吪
Merged
sapphi-red merged 5 commits intooxc-project:mainfrom Apr 20, 2026
Merged
fix(minifier): mark LHS of ??= as read when converting from == null &吪sapphi-red merged 5 commits intooxc-project:mainfrom
??= as read when converting from == null &吪sapphi-red merged 5 commits intooxc-project:mainfrom
Conversation
The `if (x == null) { x = expr; }` guard is incorrectly stripped by the
minifier when `CompressOptions::smallest()` (unused removal) is active.
After `a == null && (a = b)` is converted to `a ??= b`, the LHS
reference is still flagged as Write-only; on the next iteration the
unused-removal pass sees `read_references_count == 0` and strips the
assignment, leaving just the RHS.
Covers both `== null &&` and `!= null ||` shapes, the lowered logical
form, and member-LHS (which goes through a different path and is
unaffected). Each case is a separate test so one regression doesn't
mask the others.
…l &&` (oxc-project#21457) When converting `a == null && (a = b)` to `a ??= b`, the LHS reference flags were not updated to include Read. This caused the unused removal pass to see `read_references_count == 0` on the next iteration and strip the assignment, removing the nullish guard entirely — e.g. `let rafId; function foo() { if (rafId == null) { rafId = raf(cb); } }` would be minified to `function foo() { raf(cb); }`, losing the "schedule at most once" semantics. The equivalent `a = a || b` → `a ||= b` and `a = a + b` → `a += b` conversions already update reference flags via `mark_assignment_target_as_read`; this patch applies the same treatment to the `??=` path.
Conflict === `crates/oxc_minifier/src/peephole/remove_unused_expression.rs` Both sides modified the same `a == null && (a = b)` → `a ??= b` branch: - HEAD (this PR) added a `Self::mark_assignment_target_as_read(&assignment_expr.left, ctx)` call right after switching the operator to `LogicalNullish`, so the next unused-removal pass doesn't see the LHS as unread and strip the whole assignment. - `origin/main` restructured the surrounding match so the `ES2020NullishCoalescingOperator` check is an arm guard (`if ... =>`) instead of a nested `if`, which dedented the inner block by one level. Resolution === Took `origin/main`'s dedented indentation and kept HEAD's new `mark_assignment_target_as_read` call (collapsed to a single line since it now fits at the shallower indent). Full minifier test suite passes (482 tests, including the five new regression tests).
Merging this PR will not alter performance
Comparing 
Footnotes
|
Contributor
Author
|
Implemented with Claude Code, then reviewed and iterated. |
sapphi-red
reviewed
Apr 20, 2026
Signed-off-by: 翠 <green@sapphi.red>
camc314
added a commit
that referenced
this pull request
Apr 20, 2026
### 🐛 Bug Fixes - 48967e8 isolated_declarations: Drop required type check for private parameter properties on private constructors (#21515) (Dunqing) - 91e5bde transformer/typescript: Preserve computed-key static block when class has an empty constructor (#21562) (Dunqing) - 50e9d26 mangler: Assign correct slot to shadowed function-expression names (#21535) (Dunqing) - 065ce47 isolated_declarations: Collect types from private accessors for paired inference (#21516) (Dunqing) - 00fc136 codegen: Preserve coverage comments before object properties (#21312) (bab) - d676e0c minifier: Mark LHS of `??=` as read when converting from `== null &&` (#21546) (Gunnlaugur Thor Briem) ### ⚡ Performance - e45efc5 parser: Reduce `try_parse` usage in favour of `lookahead` (#21532) (Boshen) - ddb1bf8 parser: Avoid redundant `IdentifierReference` clone in shorthand property (#21511) (Boshen) - be2b392 allocator: Store pointers directly in `Arena` (#21483) (overlookmotel) Co-authored-by: Boshen <1430279+Boshen@users.noreply.github.com> Co-authored-by: Cameron <cameron.clark@hey.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Fix bug: minifier drops the guard and assignment from
if (x == null) { x = expr; }, making it justexprwhich gets evaluated unconditionally.Fixes #21457
Example
was being minified down to roughly:
... losing the "schedule at most once" semantics.
Root cause
In converting
a == null && (a = b)toa ??= b, the LHS reference was left flagged Write-only. So on the next unused-removal iteration the variable appears unread. So the assignment (and thus the nullish guard inherent in it) gets stripped, leaving just the RHS.Fix
After swapping
AssignmentOperator::AssigntoAssignmentOperator::LogicalNullishinremove_unused_expression.rs, call the existingmark_assignment_target_as_readhelper. The sibling conversions inminimize_logical_expression.rs/minimize_conditions.rs(||=,&&=,+=, …) already do this — only the??=path in this file was missed.Tests
Five regression tests covering the four source shapes that lower to
??=(== null &&,!= null ||,if (x == null),if (x != null) {} else), plus a test for the member-access LHS case (e.g.o.y ??= expr), which goes through a different code path and was never affected by the bug — included to lock in that behavior so it can't regress silently. Each case is a separate#[test]so one regression does not mask the others.