Skip to content

Conversation

@delino
Copy link
Contributor

@delino delino bot commented Nov 3, 2025

Summary

This PR fixes issue #11084 where the SWC minifier incorrectly optimized array destructuring assignments, causing the minifier to replace valid assignment targets with literal values, resulting in a SyntaxError.

Problem

When minifying code with array destructuring assignments like:

const bin = { hasMore: false, hasDisorder: false };
[bin.hasMore, bin.hasDisorder] = [true, true];

The minifier incorrectly outputs:

[!1, !1] = [!0, !0];

This produces a "SyntaxError: Invalid destructuring assignment target" because the left side becomes literal boolean values instead of valid assignment targets.

Root Cause

The bug is in eval_array_spread() function (crates/swc_ecma_minifier/src/compress/pure/evaluate.rs:71-110). This function performs array optimization but did not check the context flags to determine if the array is being used in an assignment context where optimization should be skipped.

Solution

Added a context check at the beginning of eval_array_spread() to prevent optimization when the array is:

  • On the left-hand side of an assignment (IS_LHS_OF_ASSIGN)
  • In a delete operation context (IN_DELETE)
  • An argument to update operators ++/-- (IS_UPDATE_ARG)

This follows the same pattern used by other evaluation functions in the codebase, such as eval_array_method_call() at line 188-199.

Changes

  1. Source Fix: Added context check to eval_array_spread() in crates/swc_ecma_minifier/src/compress/pure/evaluate.rs
  2. Test Coverage: Added comprehensive test cases in crates/swc_ecma_minifier/tests/fixture/issues/11084/:
    • Object property destructuring assignment
    • Array element destructuring assignment
    • Nested object destructuring
    • Mixed literals and expressions

Test Plan

Related Issues

Closes #11084

🤖 Generated with Claude Code

…ent contexts

This fixes issue #11084 where the minifier incorrectly optimized array
expressions used as assignment targets in destructuring assignments,
causing them to be replaced with literal values.

Example of the bug:
```javascript
const bin = { hasMore: false, hasDisorder: false };
[bin.hasMore, bin.hasDisorder] = [true, true];
// Was incorrectly minified to: [!1, !1] = [!0, !0]
// Which produces: SyntaxError: Invalid destructuring assignment target
```

The fix adds a context check to `eval_array_spread()` to skip
optimization when the array is used:
- As the left-hand side of an assignment (IS_LHS_OF_ASSIGN)
- In a delete operation (IN_DELETE)
- As an argument to update operators like ++ or -- (IS_UPDATE_ARG)

This follows the same pattern used by other evaluation functions in the
codebase, such as `eval_array_method_call()`.

Closes #11084

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
@CLAassistant
Copy link

CLAassistant commented Nov 3, 2025

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
1 out of 2 committers have signed the CLA.

✅ kdy1
❌ github-actions[bot]
You have signed the CLA already but the status is still pending? Let us recheck it.

@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@changeset-bot
Copy link

changeset-bot bot commented Nov 3, 2025

🦋 Changeset detected

Latest commit: 2ccefa2

The changes in this PR will be included in the next version bump.

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

Copy link
Member

kdy1 commented Nov 3, 2025

🤖 This pull request has been linked to DevBird Task #1193

View the task details and manage the automated development workflow in DevBird.

Learn more about DevBird here or the announcement blog post here.

Copy link
Member

kdy1 commented Nov 3, 2025

📋 DevBird Task Prompt

Objective

Fix the bug in SWC minifier where array destructuring assignments are incorrectly optimized, causing the minifier to replace valid assignment targets with literal values, resulting in a SyntaxError.

Issue Details

GitHub Issue: #11084

Problem: When minifying code with array destructuring assignments like:

const bin = { hasMore: false, hasDisorder: false };
[bin.hasMore, bin.hasDisorder] = [true, true];

The minifier incorrectly outputs:

[!1, !1] = [!0, !0];

This produces a "SyntaxError: Invalid destructuring assignment target" because the left side becomes literal boolean values instead of valid assignment targets.

Root Cause Analysis

The bug is in crates/swc_ecma_minifier/src/compress/pure/evaluate.rs at lines 71-110 in the eval_array_spread() function. This function performs array optimization but does not check the context flags to determine if the array is being used in an assignment context where optimization should be skipped.

Similar functions that correctly check context:

  • eval_array_method_call() at line 188-199 checks IS_LHS_OF_ASSIGN context
  • eval_str_method_call() at line 826-838 checks IS_LHS_OF_ASSIGN context
  • Many other eval functions follow this pattern

Technical Requirements

Documentation & Resources

Code Location

  • File: crates/swc_ecma_minifier/src/compress/pure/evaluate.rs
  • Function: eval_array_spread() (lines 71-110)
  • Context Flags: Defined in crates/swc_ecma_minifier/src/compress/pure/ctx.rs

Implementation Steps

  1. Add context check to eval_array_spread():
    Add the following check at the beginning of the function (after the evaluate option check):

    if self.ctx.intersects(
        Ctx::IN_DELETE
            .union(Ctx::IS_UPDATE_ARG)
            .union(Ctx::IS_LHS_OF_ASSIGN),
    ) {
        return;
    }

    This prevents array optimization when the array is:

    • In a delete operation context
    • An argument to update operators (++/--)
    • On the left-hand side of an assignment
  2. Create comprehensive test cases:
    Create a new test fixture at crates/swc_ecma_minifier/tests/fixture/issues/11084/ with:

    input.js:

    // Test case 1: Object property destructuring assignment
    const bin = { hasMore: false, hasDisorder: false };
    [bin.hasMore, bin.hasDisorder] = [true, true];
    console.log(bin.hasMore, bin.hasDisorder);
    
    // Test case 2: Array element destructuring assignment
    const arr = [1, 2];
    [arr[0], arr[1]] = [arr[1], arr[0]];
    console.log(arr);
    
    // Test case 3: Nested object destructuring
    const obj = { a: { x: 0 }, b: { y: 0 } };
    [obj.a.x, obj.b.y] = [10, 20];
    console.log(obj.a.x, obj.b.y);
    
    // Test case 4: Mixed literals and expressions
    const state = { flag: false };
    [state.flag] = [true];
    console.log(state.flag);

    config.json:

    {
      "compress": {
        "booleans": true,
        "properties": true,
        "evaluate": true
      }
    }

    The output should preserve the destructuring assignment structure without optimizing the left-hand side expressions.

  3. Verify existing related tests still pass:

    • Check tests/fixture/issues/9739/ - similar array destructuring bug
    • Check terser compatibility tests for destructuring in tests/terser/compress/collapse_vars/
  4. Run the full test suite:

    cd crates/swc_ecma_minifier
    cargo test
  5. Format code according to project standards:

    cargo fmt --all

Success Criteria

  • The eval_array_spread() function correctly checks context flags before optimization
  • All new test cases pass, showing correct handling of destructuring assignments
  • The minifier preserves assignment target expressions without substituting values
  • All existing minifier tests continue to pass
  • Code is properly formatted with cargo fmt

Project-Specific Requirements (from CLAUDE.md)

  1. Write performant code - the fix should not impact minification performance
  2. Write comments in English explaining the context check
  3. Do not use unstable/nightly-only Rust features
  4. Write unit tests for the code (as outlined above)
  5. Do not remove or modify existing tests
  6. Write documentation/comments for the fix
  7. Run cargo fmt --all before committing

Context References

  • The fix should follow the same pattern as other evaluation functions in the same file
  • The IS_LHS_OF_ASSIGN context flag is already set correctly in visit_mut_assign_expr() at line 199-225 of crates/swc_ecma_minifier/src/compress/pure/mod.rs
  • This is a defensive fix that prevents optimization in contexts where it could break JavaScript semantics

This comment was automatically added by DevBird. You can disable this feature in DevBird Settings.

@claude
Copy link
Contributor

claude bot commented Nov 3, 2025

Claude finished @DeLino[bot]'s task —— View job


Code Review for Array Destructuring Minifier Fix

Reviewing the pull request for issue #11084...

Todo List:

  • Read and analyze the core fix in evaluate.rs
  • Review test coverage and test cases
  • Check code quality and best practices
  • Look for potential bugs or issues
  • Assess performance considerations
  • Check for security concerns
  • Provide comprehensive feedback

✅ Overall Assessment

This is a well-implemented fix that correctly addresses the array destructuring optimization bug. The solution follows established patterns in the codebase and includes comprehensive test coverage.

🔍 Code Quality Analysis

Core Fix (evaluate.rs:71-86)

  • Correctly implemented: The context check follows the exact same pattern as eval_array_method_call() at lines 205-211
  • Appropriate placement: Added at the function start after the evaluate option check, preventing unnecessary work
  • Proper context flags: Uses the right combination of IN_DELETE, IS_UPDATE_ARG, and IS_LHS_OF_ASSIGN

⚠️ Comment Issue (Minor)

The comment at lines 78-79 is misleading as noted by the copilot reviewer:

// This prevents invalid transformations like: [obj.prop] = [true] => [!0] = [!0]

The actual problematic transformation would be [obj.prop] = [true] becoming [!1] = [!0] where the LHS gets optimized to literals. The comment suggests both sides get transformed, but the LHS should never be optimized.

🧪 Test Coverage - Excellent

The test cases comprehensively cover the bug scenarios:

  1. Object property destructuring: [bin.hasMore, bin.hasDisorder] = [true, true]
  2. Array element swapping: [arr[0], arr[1]] = [arr[1], arr[0]]
  3. Nested object properties: [obj.a.x, obj.b.y] = [10, 20]
  4. Mixed scenarios: [state.flag] = [true]

The expected output correctly shows:

  • ✅ LHS expressions preserved: [bin.hasMore, bin.hasDisorder] stays intact
  • ✅ RHS optimizations still work: [true, true] becomes [!0, !0]
  • ✅ Other optimizations unaffected: Object literals still get boolean optimization

Performance Considerations

  • Minimal overhead: Early return prevents any processing when in assignment contexts
  • No performance regression: The context check is O(1) bitwise operation
  • Preserves other optimizations: Only blocks array optimization in specific contexts

🔒 Security Assessment

  • No security concerns: This is a defensive fix that prevents invalid JavaScript generation
  • Follows principle of least privilege: Only disables optimization in problematic contexts

🏗️ Architecture Consistency

  • Follows established patterns: Identical to other eval_* functions in the same file
  • Proper context usage: Uses the same context flags as designed for this purpose
  • Maintains separation of concerns: Fix is isolated to the specific optimization function

📋 Recommendations

  1. Fix the misleading comment (lines 78-79):

    // This prevents invalid transformations like: [obj.prop] = [true] => [!1] = [!0]
    // where the LHS gets incorrectly optimized to literals, causing SyntaxError
  2. Consider adding edge case tests (optional):

    • Nested arrays: [[obj.prop]] = [[true]]
    • Complex member expressions: [obj['key']] = [value]

🎯 Verdict

APPROVE - This fix correctly solves the reported issue with minimal risk and excellent test coverage. The implementation follows established codebase patterns and maintains backward compatibility while preventing the specific syntax error scenario.

The only issue is a minor comment correction that doesn't affect functionality.


@github-actions
Copy link
Contributor

github-actions bot commented Nov 3, 2025

Binary Sizes

File Size
swc.linux-x64-gnu.node 31M (31907080 bytes)

Commit: 719deea

@codspeed-hq
Copy link

codspeed-hq bot commented Nov 3, 2025

CodSpeed Performance Report

Merging #11221 will not alter performance

Comparing devbird/fix-array-destructuring-minifier-11084 (2ccefa2) with main (af134fa)1

Summary

✅ 138 untouched

Footnotes

  1. No successful run was found on main (9436f07) during the generation of this report, so af134fa was used instead as the comparison base. There might be some changes unrelated to this pull request in this report.

@kdy1 kdy1 added this to the Planned milestone Nov 3, 2025
Update the expected test output to match the actual formatting produced by the minifier. The fix is working correctly - it preserves assignment targets on the left-hand side of destructuring assignments and doesn't optimize them away. The only difference was in the code formatting (multi-line vs single-line array literals).

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
@kdy1 kdy1 marked this pull request as ready for review November 3, 2025 08:14
@kdy1 kdy1 requested a review from a team as a code owner November 3, 2025 08:14
Copilot AI review requested due to automatic review settings November 3, 2025 08:14
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR fixes a minification bug where array literals used as left-hand side (LHS) of destructuring assignments were incorrectly being optimized. The fix prevents the minifier from transforming boolean literals within arrays when those arrays are assignment targets.

  • Added guard to prevent optimization of arrays used as assignment targets in destructuring patterns
  • Added test case covering various destructuring assignment scenarios
  • Ensures expressions like [obj.prop] = [true] are not incorrectly transformed to [!0] = [!0]

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File Description
crates/swc_ecma_minifier/src/compress/pure/evaluate.rs Added context check to prevent array optimization when used as LHS of assignments
crates/swc_ecma_minifier/tests/fixture/issues/11084/input.js Input test file with destructuring assignment patterns
crates/swc_ecma_minifier/tests/fixture/issues/11084/output.js Expected output demonstrating correct handling of destructuring assignments
crates/swc_ecma_minifier/tests/fixture/issues/11084/config.json Test configuration enabling booleans, properties, and evaluate optimizations

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +80 to +86
if self.ctx.intersects(
Ctx::IN_DELETE
.union(Ctx::IS_UPDATE_ARG)
.union(Ctx::IS_LHS_OF_ASSIGN),
) {
return;
}
Copy link

Copilot AI Nov 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comment on line 76-79 states this prevents transformations like '[obj.prop] = [true] => [!0] = [!0]', but this example is misleading. The actual problematic transformation would be '[obj.prop] = [true] => [obj.prop] = [!0]' where the right-hand side array's boolean literal gets optimized. The left-hand side array should never be transformed to '[!0]' as it's an assignment target, not a literal.

Copilot uses AI. Check for mistakes.
@kdy1 kdy1 requested a review from a team as a code owner November 4, 2025 06:17
@kdy1 kdy1 merged commit 99d8b0a into main Nov 4, 2025
183 of 184 checks passed
@kdy1 kdy1 deleted the devbird/fix-array-destructuring-minifier-11084 branch November 4, 2025 08:09
@kdy1 kdy1 modified the milestones: Planned, 1.15.0 Nov 10, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

Left-hand side with object attributes substituted by value in array destructuring

3 participants