Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(es/minifier): Preserve flags while dropping elements of SeqExpr #8907

Merged
merged 16 commits into from
Jul 31, 2024

Conversation

devongovett
Copy link
Contributor

In code like /*#__PURE__*/ (0, forwardRef)(Foo);, the dead_branch_remover pass will simplify this to forwardRef(Foo) and remove the PURE annotation which might be used in later passes. I added a test to show this, and commented out the line that causes it. However I'm not quite sure how to fix it.

I guess the comment is attached to the original sequence expression which was removed. So we need to move the comment to the resulting expression somehow, or maybe change the span of the expression to match the old one.

I tried passing in the comments to this pass so I could move it, but it was a bit hard because of the Parallel implementation that would require the comments to be Send + Sync.

Any suggestions on the best approach here?

Copy link

changeset-bot bot commented May 1, 2024

🦋 Changeset detected

Latest commit: 3e03eb6

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

// TODO: preserve PURE comment attached to original
// sequence expression.

// *e = *s.exprs.pop().unwrap();
Copy link
Member

Choose a reason for hiding this comment

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

You can

  1. assign the span of the node with PURE comment to the emitted node

  2. Accept C: Comments or &dyn Comments and call move_leading

@kdy1 kdy1 self-assigned this Jun 22, 2024
@kdy1 kdy1 added this to the Planned milestone Jun 22, 2024
@kdy1
Copy link
Member

kdy1 commented Jun 22, 2024

I'll take on this

kdy1 added a commit that referenced this pull request Jul 6, 2024
**Description:**

This is required for #8907
@kdy1 kdy1 changed the title dead_branch_remover removes PURE annotations on sequence expressions fix(es/minifier): Preserve flags while dropping elements of SeqExpr Jul 31, 2024
@kdy1 kdy1 marked this pull request as ready for review July 31, 2024 07:26
@kdy1 kdy1 requested review from a team as code owners July 31, 2024 07:26
@kdy1 kdy1 enabled auto-merge (squash) July 31, 2024 07:28
@kdy1 kdy1 merged commit 24e8798 into swc-project:main Jul 31, 2024
154 checks passed
Copy link

codspeed-hq bot commented Jul 31, 2024

CodSpeed Performance Report

Merging #8907 will improve performances by 8.8%

Comparing devongovett:branch-simplify-pure (3e03eb6) with devongovett:branch-simplify-pure (f09fd9c)

Summary

⚡ 1 improvements
✅ 177 untouched benchmarks

Benchmarks breakdown

Benchmark devongovett:branch-simplify-pure devongovett:branch-simplify-pure Change
es/full/bugs-1 580.4 µs 533.5 µs +8.8%

@kdy1 kdy1 modified the milestones: Planned, v1.7.4 Jul 31, 2024
@swc-project swc-project locked as resolved and limited conversation to collaborators Aug 31, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants