fix(minifier): disallow merging assignments to let declarations when TDZ error would be introduced#13635
Conversation
How to use the Graphite Merge QueueAdd either label to this PR to merge it via the merge queue:
You must have a Graphite account in order to use the merge queue. Sign up using this link. An organization admin has enabled the Graphite Merge Queue in this repository. Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue. This stack of pull requests is managed by Graphite. Learn more about stacking. |
5d597f7 to
2c8e941
Compare
CodSpeed Instrumentation Performance ReportMerging #13635 will not alter performanceComparing Summary
Footnotes |
There was a problem hiding this comment.
Pull Request Overview
This PR fixes a bug in the minifier where merging assignments to let declarations could introduce Temporal Dead Zone (TDZ) errors. The fix prevents merging when the assignment's right-hand side is not a literal value for let declarations, ensuring that expressions like let x; x = (() => x?.a)() are not incorrectly transformed to let x = (() => x?.a)().
- Added safety check to prevent TDZ errors when merging assignments to
letdeclarations - Restricted merging to literal values only for
letdeclarations while keeping existing behavior forvar - Added comprehensive test cases covering various TDZ scenarios
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| crates/oxc_minifier/src/peephole/minimize_statements.rs | Core logic fix adding TDZ safety check for let declarations |
| crates/oxc_minifier/tests/peephole/merge_assignments_to_declarations.rs | Test cases covering TDZ error scenarios |
| tasks/track_memory_allocations/allocs_minifier.snap | Memory allocation tracking update reflecting the code changes |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
Merge activity
|
…TDZ error would be introduced (#13635) ```js let x; x = (() => x?.a)(); ``` should not be transformed to ```js let x = (() => x?.a)(); ``` , as it would introduce a TDZ error. To fix that, I limited the right hand side of the assignment to be a literal value for let declarations. The precise check would be to check whether the right hand side includes a reference to the variable declared on the left hand side, but I didn't do that for now as it is complicated. If we implement that, we can improve this condition too. https://github.com/oxc-project/oxc/blob/5d597f71c06a9c56fe404a712c05b61e2da3697a/crates/oxc_minifier/src/peephole/minimize_statements.rs#L544-L548 fixes #13615 refs #13270
2c8e941 to
f9fd65b
Compare

should not be transformed to
, as it would introduce a TDZ error.
To fix that, I limited the right hand side of the assignment to be a literal value for let declarations.
The precise check would be to check whether the right hand side includes a reference to the variable declared on the left hand side, but I didn't do that for now as it is complicated. If we implement that, we can improve this condition too.
oxc/crates/oxc_minifier/src/peephole/minimize_statements.rs
Lines 544 to 548 in 5d597f7
fixes #13615
refs #13270