Skip to content

fix(es/transforms): rewrite class references in non-static members#11772

Merged
kdy1 merged 3 commits intomainfrom
04-07-fix/11744
Apr 7, 2026
Merged

fix(es/transforms): rewrite class references in non-static members#11772
kdy1 merged 3 commits intomainfrom
04-07-fix/11744

Conversation

@CPunisher
Copy link
Copy Markdown
Member

@CPunisher CPunisher commented Apr 7, 2026

Description:

Decorated class declarations with lifted static members failed to rewrite class self-references inside instance methods. As a result, instance methods could still read the pre-decoration class binding and observe undefined instead of the decorated class’s static fields.

Related issue (if exists):

fixes #11744

@changeset-bot
Copy link
Copy Markdown

changeset-bot bot commented Apr 7, 2026

🦋 Changeset detected

Latest commit: 90e1c27

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

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 7, 2026

Binary Sizes

File Size
swc.linux-x64-gnu.node 28M (29074376 bytes)

Commit: 65ebca5

@codspeed-hq
Copy link
Copy Markdown

codspeed-hq bot commented Apr 7, 2026

Merging this PR will not alter performance

✅ 219 untouched benchmarks


Comparing 04-07-fix/11744 (90e1c27) with main (236eff0)1

Open in CodSpeed

Footnotes

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

@CPunisher CPunisher marked this pull request as ready for review April 7, 2026 09:55
@CPunisher CPunisher requested a review from a team as a code owner April 7, 2026 09:55
Copilot AI review requested due to automatic review settings April 7, 2026 09:55
@claude

This comment has been minimized.

Copy link
Copy Markdown
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

Fixes a decorators (2022-03) transformation bug where class self-references inside non-static members were not rewritten when static members are lifted, causing instance methods to read the pre-decoration class binding and observe undefined instead of the decorated class’s static fields.

Changes:

  • Rewrite class self-references inside all class methods/private methods (not just static ones) during the lifted-static-member path of the decorator transform.
  • Add a regression fixture for issue #11744, covering the instance-method ScrollView.scrollInterval access case.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.

File Description
crates/swc_ecma_transforms_proposal/src/decorator_impl.rs Extends replace_ident rewriting to non-static Method/PrivateMethod members so instance methods refer to the decorated class binding.
crates/swc/tests/fixture/issues-11xxx/11744/input/index.js Adds minimal repro input for #11744.
crates/swc/tests/fixture/issues-11xxx/11744/input/.swcrc Configures decorators 2022-03 transform for the fixture.
crates/swc/tests/fixture/issues-11xxx/11744/output/index.js Updates expected output to ensure instance method references rewritten to _ScrollView.

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

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 13d5b93204

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +1759 to 1760
ClassMember::Method(method) => {
replace_ident(method, c.ident.to_id(), &new_class_name);
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Skip renaming class refs in instance method keys

Removing the is_static guard here makes replace_ident run on the entire ClassMethod, which includes computed keys. For decorated class declarations with static members, an instance method like [@dec class C { static x = 1; [C]() {} }] now has its key rewritten to [_C], but _C is assigned later in the decorator init flow, so the computed key evaluates to undefined instead of the class reference. This regression is introduced by broadening the match from static-only methods to all methods.

Useful? React with 👍 / 👎.

@kdy1 kdy1 requested a review from a team as a code owner April 7, 2026 17:49
Copy link
Copy Markdown
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.

Thanks!

@kdy1 kdy1 merged commit fff1426 into main Apr 7, 2026
42 checks passed
@kdy1 kdy1 deleted the 04-07-fix/11744 branch April 7, 2026 17:50
@github-actions github-actions bot added this to the Planned milestone Apr 7, 2026
@claude
Copy link
Copy Markdown
Contributor

claude bot commented Apr 7, 2026

PR Review: fix(es/transforms): rewrite class references in non-static members

Summary

Clean, minimal fix for #11744. The bug was that replace_ident (which rewrites class self-references like ScrollView_ScrollView) was only applied to static methods, but instance methods can also reference the class by name (e.g., ScrollView.scrollInterval inside an instance method). The fix correctly removes the if method.is_static guard so all methods get the rewrite.

Feedback

Code change — Looks correct. The other arms in the same match (ClassProp, PrivateProp, AutoAccessor) already applied replace_ident unconditionally (no static check), so restricting Method and PrivateMethod to static-only was inconsistent. This fix makes the behavior uniform across all member types. 👍

Test coverage — The fixture test reproduces the exact scenario from the issue (decorated class with a static field referenced from an instance method). The expected output correctly shows _ScrollView.scrollInterval inside autoScroll(). Good.

Minor nit — The input file input/index.js is missing a trailing newline (see \ No newline at end of file in the diff). This is cosmetic but most fixtures in the repo have trailing newlines. Not a blocker.

No concerns on performance or security — the change only removes a guard condition, so it's strictly additive in what gets processed and the operation (replace_ident) is already applied to the same node types via adjacent arms.

Overall this is a well-scoped, correct fix. LGTM.

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.

Incorrect decorator transformation

3 participants