Skip to content

feat(es/transforms): Add renamer_keep_contexts #10907

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 7 commits into from
Jul 22, 2025
Merged

feat(es/transforms): Add renamer_keep_contexts #10907

merged 7 commits into from
Jul 22, 2025
+269 −87

Conversation

mischnic
Copy link
Contributor

@mischnic mischnic commented Jul 21, 2025 •
edited
Loading

Description:

When using renamer outside of hygiene (i.e. you want to continue using the AST), you want correct context contexts in some cases.

BREAKING CHANGE:

Related issue (if exists):

@changeset-bot changeset-bot
Copy link

changeset-bot bot commented Jul 21, 2025 •
edited
Loading

🦋 Changeset detected

Latest commit: ee80f5c

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

@codspeed-hq CodSpeed HQ
Copy link

codspeed-hq bot commented Jul 21, 2025 •
edited
Loading

CodSpeed Performance Report

Merging #10907 will not alter performance

Comparing mischnic:mischnic/hygiene-stay-unique (ee80f5c) with main (6f5b72a)

Summary

✅ 140 untouched benchmarks

@mischnic mischnic force-pushed the mischnic/hygiene-stay-unique branch 3 times, most recently from f8b79e7 to 0a260ab Compare July 21, 2025 11:24
mischnic
mischnic commented Jul 21, 2025
View reviewed changes
@mischnic mischnic changed the title renamer: Always assign correct contexts feat(): Add renamer_keep_contexts Jul 21, 2025
@kdy1 kdy1 changed the base branch from main to dev/rust July 21, 2025 11:29
@kdy1 kdy1 force-pushed the dev/rust branch 4 times, most recently from 3416769 to 9703599 Compare July 22, 2025 04:32
@kdy1 kdy1 deleted the branch swc-project:main July 22, 2025 05:21
@kdy1 kdy1 closed this Jul 22, 2025
@kdy1 kdy1 reopened this Jul 22, 2025
@kdy1 kdy1 changed the base branch from dev/rust to main July 22, 2025 05:47
@mischnic mischnic force-pushed the mischnic/hygiene-stay-unique branch from 0a260ab to 00d8d09 Compare July 22, 2025 08:19
@mischnic mischnic force-pushed the mischnic/hygiene-stay-unique branch from 00d8d09 to f98cad2 Compare July 22, 2025 08:25
@mischnic mischnic changed the title feat(): Add renamer_keep_contexts feat(es/transforms): Add renamer_keep_contexts Jul 22, 2025
@mischnic mischnic marked this pull request as ready for review July 22, 2025 08:26
@mischnic mischnic requested a review from a team as a code owner July 22, 2025 08:26
@mischnic mischnic requested a review from kdy1 July 22, 2025 08:26
kdy1
kdy1 reviewed Jul 22, 2025
View reviewed changes
kdy1
kdy1 reviewed Jul 22, 2025
View reviewed changes
Copy link
Member

@kdy1 kdy1 left a comment

Choose a reason for hiding this comment

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

I think I need to configure a binary size CI action before merging this PR.
I'll copy one from rspack later this week

kdy1
kdy1 reviewed Jul 22, 2025
View reviewed changes
Copy link
Member

@kdy1 kdy1 left a comment

Choose a reason for hiding this comment

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

I think I need to configure a binary size CI action before merging this PR.
I'll copy one from rspack later this week

@kdy1 kdy1 requested a review from a team as a code owner July 22, 2025 08:52
@kdy1
Copy link
Member

kdy1 commented Jul 22, 2025

In case you have a local patch of SWC for turbopack, can you profile the binary size of turbopack with this patch?

@mischnic
Copy link
Contributor Author

I can't measure a difference:

swc/main 4eab8e8:

$ ll packages/next-swc/native/next-swc.darwin-arm64.node
-rwxr-xr-x  1 niklas  staff   214M Jul 22 11:09 packages/next-swc/native/next-swc.darwin-arm64.node*
$ strip -x packages/next-swc/native/next-swc.darwin-arm64.node
$ ll packages/next-swc/native/next-swc.darwin-arm64.node
-rwxr-xr-x  1 niklas  staff   125M Jul 22 11:09 packages/next-swc/native/next-swc.darwin-arm64.node*

my fix f98cad2 but without any code change on Turbopack side:

$ ll packages/next-swc/native/next-swc.darwin-arm64.node
-rwxr-xr-x  1 niklas  staff   214M Jul 22 11:13 packages/next-swc/native/next-swc.darwin-arm64.node*
$ strip -x packages/next-swc/native/next-swc.darwin-arm64.node
$ ll packages/next-swc/native/next-swc.darwin-arm64.node
-rwxr-xr-x  1 niklas  staff   125M Jul 22 11:14 packages/next-swc/native/next-swc.darwin-arm64.node*

my fix f98cad2 and using renamer_keep_contexts:

$ ll packages/next-swc/native/next-swc.darwin-arm64.node
-rwxr-xr-x  1 niklas  staff   214M Jul 22 11:16 packages/next-swc/native/next-swc.darwin-arm64.node*
$ strip -x packages/next-swc/native/next-swc.darwin-arm64.node
$ ll packages/next-swc/native/next-swc.darwin-arm64.node
-rwxr-xr-x  1 niklas  staff   125M Jul 22 11:18 packages/next-swc/native/next-swc.darwin-arm64.node*

kdy1
kdy1 reviewed Jul 22, 2025
View reviewed changes
@mischnic
Copy link
Contributor Author

mischnic commented Jul 22, 2025 •
edited
Loading

Also, there is no overhead at all unless you're actually using renamer_keep_contexts because it compiles to the same code as before if you're only using type Target = Atom

kdy1
kdy1 reviewed Jul 22, 2025
View reviewed changes
Copy link
Member

@kdy1 kdy1 left a comment

Choose a reason for hiding this comment

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

Can you write some descriptions as comments? (in the source code)_

crates/swc_ecma_transforms_base/src/hygiene/tests.rs
)?);

let expected = "
const bar$1 = (patch1$7)=>patch1$7 + 1;
Copy link
Member

Choose a reason for hiding this comment

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

I don't get why this is $7.

@kdy1 kdy1 enabled auto-merge (squash) July 22, 2025 13:06
@kdy1 kdy1 disabled auto-merge July 22, 2025 13:07
@kdy1 kdy1 enabled auto-merge (squash) July 22, 2025 13:07
@kdy1 kdy1 merged commit 1b15171 into swc-project:main Jul 22, 2025
174 checks passed
@kdy1 kdy1 added this to the v1.13.2 milestone Jul 22, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kdy1 kdy1 kdy1 approved these changes

Assignees
No one assigned
Labels
None yet
Milestone
v1.13.2
Development

Successfully merging this pull request may close these issues.
2 participants
@mischnic @kdy1