Uh oh!
There was an error while loading. Please reload this page.
feat(swc): dead code eliminate unreferenced private class members in workflow mode - #1671
Merged
Conversation
…workflow mode After stripping 'use step' methods from a class body in workflow mode, eliminate private members (both JS native #field/#method and TypeScript private field/private method) that are no longer referenced by any remaining public member. The algorithm is iterative: references are seeded from public members, then expanded through surviving private members until a fixed point, enabling cascading elimination (e.g. a private field only referenced by a private method that is itself unreferenced).
🦋 Changeset detectedLatest commit: 4ea576a The changes in this PR will be included in the next version bump. This PR includes changesets to release 17 packages
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 |
Contributor
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
🧪 E2E Test Results❌ Some tests failed Summary
❌ Failed Tests🌍 Community Worlds (65 failed)mongodb (4 failed):
redis (3 failed):
turso (58 failed):
Details by Category✅ ▲ Vercel Production
✅ 💻 Local Development
✅ 📦 Local Production
✅ 🐘 Local Postgres
✅ 🪟 Windows
❌ 🌍 Community Worlds
✅ 📋 Other
|
Contributor
📊 Benchmark Results
workflow with no steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Express | Next.js (Turbopack) | Nitro workflow with 1 step💻 Local Development
▲ Production (Vercel)
🔍 Observability: Express | Nitro | Next.js (Turbopack) workflow with 10 sequential steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Express | Next.js (Turbopack) | Nitro workflow with 25 sequential steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Express | Next.js (Turbopack) | Nitro workflow with 50 sequential steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Next.js (Turbopack) | Express | Nitro Promise.all with 10 concurrent steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Express | Nitro | Next.js (Turbopack) Promise.all with 25 concurrent steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Nitro | Next.js (Turbopack) | Express Promise.all with 50 concurrent steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Express | Nitro | Next.js (Turbopack) Promise.race with 10 concurrent steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Express | Nitro | Next.js (Turbopack) Promise.race with 25 concurrent steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Express | Nitro | Next.js (Turbopack) Promise.race with 50 concurrent steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Nitro | Express | Next.js (Turbopack) workflow with 10 sequential data payload steps (10KB)💻 Local Development
▲ Production (Vercel)
🔍 Observability: Nitro | Next.js (Turbopack) | Express workflow with 25 sequential data payload steps (10KB)💻 Local Development
▲ Production (Vercel)
🔍 Observability: Nitro | Express | Next.js (Turbopack) workflow with 50 sequential data payload steps (10KB)💻 Local Development
▲ Production (Vercel)
🔍 Observability: Express | Nitro | Next.js (Turbopack) workflow with 10 concurrent data payload steps (10KB)💻 Local Development
▲ Production (Vercel)
🔍 Observability: Express | Next.js (Turbopack) | Nitro workflow with 25 concurrent data payload steps (10KB)💻 Local Development
▲ Production (Vercel)
🔍 Observability: Nitro | Express | Next.js (Turbopack) workflow with 50 concurrent data payload steps (10KB)💻 Local Development
▲ Production (Vercel)
🔍 Observability: Nitro | Next.js (Turbopack) | Express Stream Benchmarks(includes TTFB metrics)workflow with stream💻 Local Development
▲ Production (Vercel)
🔍 Observability: Next.js (Turbopack) | Express | Nitro stream pipeline with 5 transform steps (1MB)💻 Local Development
▲ Production (Vercel)
🔍 Observability: Express | Next.js (Turbopack) | Nitro 10 parallel streams (1MB each)💻 Local Development
▲ Production (Vercel)
🔍 Observability: Express | Nitro | Next.js (Turbopack) fan-out fan-in 10 streams (1MB each)💻 Local Development
▲ Production (Vercel)
🔍 Observability: Nitro | Next.js (Turbopack) | Express SummaryFastest Framework by WorldWinner determined by most benchmark wins
Fastest World by FrameworkWinner determined by most benchmark wins
Column Definitions
Worlds:
|
Contributor
There was a problem hiding this comment.
Pull request overview
Adds a workflow-mode optimization to the SWC plugin so that after "use step" class members are stripped, any now-unreferenced private class members are also removed, improving downstream tree-shaking (e.g., dropping Node-only imports kept alive by unused helpers).
Changes:
- Implement iterative private-member reachability analysis and removal after step-member stripping (class decls + class exprs).
- Add a new transform fixture validating cascading elimination of TypeScript
privatemembers in workflow mode. - Document the new optimization and publish a patch changeset.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/swc-plugin-workflow/transform/src/lib.rs | Adds private-member reference collection + DCE pass after "use step" stripping. |
| packages/swc-plugin-workflow/transform/tests/fixture/private-member-dce/input.ts | New fixture input covering cascading TS private elimination. |
| packages/swc-plugin-workflow/transform/tests/fixture/private-member-dce/output-workflow.js | Expected workflow output with TS-private members removed and imports dropped. |
| packages/swc-plugin-workflow/transform/tests/fixture/private-member-dce/output-step.js | Expected step-mode output (private members retained). |
| packages/swc-plugin-workflow/transform/tests/fixture/private-member-dce/output-client.js | Expected client-mode output (private members retained). |
| packages/swc-plugin-workflow/spec.md | Documents the private-member DCE behavior and example. |
| .changeset/private-member-dce.md | Declares a patch release for the SWC plugin change. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+619
to
+624
| // TS private or any member: `this.foo` — only track when | ||
| // the object is `this` (to avoid false positives from | ||
| // unrelated member accesses like `obj.foo`) | ||
| MemberProp::Ident(ident) => { | ||
| if matches!(&*expr.obj, Expr::This(_)) { | ||
| self.referenced.insert(ident.sym.to_string()); |
There was a problem hiding this comment.
ClassMemberRefCollector only records MemberProp::Ident accesses when the object is this. In TypeScript, private members are also legally accessed via other instances of the same class (e.g. static compare(a: C, b: C) { return a.x - b.x }). With the current check, those references won’t be collected and the private member can be incorrectly removed, breaking runtime behavior. Consider treating any .ident access as a reference when ident matches a known TS-private member name in the class, regardless of the receiver expression (safe, though more conservative).
Suggested change
| // TS private or any member: `this.foo` — only track when | |
| // the object is `this` (to avoid false positives from | |
| // unrelated member accesses like `obj.foo`) | |
| MemberProp::Ident(ident) => { | |
| ifmatches!(&*expr.obj,Expr::This(_)){ | |
| self.referenced.insert(ident.sym.to_string()); | |
| // TS private or any member access. Track `this.foo` as before, | |
| // and also track `obj.foo` when `foo` is a known TS-private | |
| // member of the current class so same-class accesses like | |
| // `a.x` / `b.x` are not missed. | |
| MemberProp::Ident(ident) => { | |
| let name = ident.sym.to_string(); | |
| ifmatches!(&*expr.obj,Expr::This(_)) || self.private_names.contains(&name){ | |
| self.referenced.insert(name); |
MemberAuthor
There was a problem hiding this comment.
Fixed. The collector now holds a ts_private_names: HashSet<String> built from the class body. MemberProp::Ident accesses are tracked when the name is in that set, regardless of the receiver expression. This catches a.x / b.x patterns in static methods.
Comment on lines
+629
to
+630
| // Continue visiting children | ||
| expr.obj.visit_with(self); |
There was a problem hiding this comment.
visit_member_expr overrides default traversal but only visits expr.obj. For computed member expressions (obj[expr]), the property expression is not visited, so any this.#x / this.x references inside the computed key won’t be collected and could cause incorrectly eliminating private members. Use expr.visit_children_with(self) (or explicitly visit the computed property expression) to preserve full traversal.
Suggested change
| // Continue visiting children | |
| expr.obj.visit_with(self); | |
| // Continue visiting children, including computed property expressions. | |
| expr.visit_children_with(self); |
MemberAuthor
There was a problem hiding this comment.
Fixed. Changed to expr.visit_children_with(self) which visits both the object and property expressions, including computed keys.
Comment on lines
+614
to
+625
| match &expr.prop { | ||
| // Native JS private: `this.#foo` | ||
| MemberProp::PrivateName(name) => { | ||
| self.referenced.insert(name.name.to_string()); | ||
| } | ||
| // TS private or any member: `this.foo` — only track when | ||
| // the object is `this` (to avoid false positives from | ||
| // unrelated member accesses like `obj.foo`) | ||
| MemberProp::Ident(ident) => { | ||
| if matches!(&*expr.obj, Expr::This(_)) { | ||
| self.referenced.insert(ident.sym.to_string()); | ||
| } |
There was a problem hiding this comment.
Native private #foo and public/TS member foo are both stored as the same string ("foo"). If a class contains both foo and #foo, a this.foo access will incorrectly mark #foo as referenced, preventing intended elimination. Consider encoding JS-private names distinctly (e.g. prefix with # or store an enum) so #foo and foo don’t collide.
MemberAuthor
There was a problem hiding this comment.
Fixed. JS native private names are now stored with a # prefix (e.g. "#foo") while TS private names remain unprefixed ("foo"). private_member_name() returns the prefixed form for PrivateMethod/PrivateProp, and visit_member_expr inserts format!("#{}", name) for MemberProp::PrivateName.
Comment on lines
+1
to
+25
| import { WORKFLOW_SERIALIZE, WORKFLOW_DESERIALIZE } from '@workflow/serde'; | ||
| import { getWorld } from './world.js'; | ||
| import { importKey } from './encryption.js'; | ||
| export class Run { | ||
| static [WORKFLOW_SERIALIZE](instance: Run) { | ||
| return { id: instance.id }; | ||
| } | ||
| static [WORKFLOW_DESERIALIZE](data: { id: string }) { | ||
| return new Run(data.id); | ||
| } | ||
| id: string; | ||
| // TS private field — only referenced by stripped methods | ||
| private encryptionKeyPromise: Promise<any> | null = null; | ||
| // TS private method — only called by stripped getters/methods | ||
| private async getEncryptionKey(): Promise<any> { | ||
| if (!this.encryptionKeyPromise) { | ||
| this.encryptionKeyPromise = importKey(this.id); | ||
| } | ||
| return this.encryptionKeyPromise; | ||
| } |
There was a problem hiding this comment.
The new DCE logic claims to support native JS private members (#field/#method()), but the fixture only covers TypeScript private. Adding a fixture that uses #private syntax (including a cascading case) would help prevent regressions in the JS-private path.
MemberAuthor
There was a problem hiding this comment.
Added in the second commit: private-member-dce-native/input.js tests JS native #field and #method() DCE including cascading elimination (#encryptionKeyPromise eliminated because #getEncryptionKey is eliminated) and survival of referenced members (#label kept because toString() references it).
Comment on lines
+8504
to
+8510
| // Dead-code-eliminate unreferenced private members | ||
| // (same logic as visit_mut_class_decl above) | ||
| let referenced = | ||
| ClassMemberRefCollector::collect_from_class_body(&class_expr.class.body); | ||
| class_expr.class.body.retain(|member| match member { | ||
| ClassMember::PrivateMethod(m) => referenced.contains(&m.key.name.to_string()), | ||
| ClassMember::PrivateProp(p) => referenced.contains(&p.key.name.to_string()), |
There was a problem hiding this comment.
The private-member retention filter is duplicated in both visit_mut_class_decl and visit_mut_class_expr. This duplication increases the risk of future divergence/bugs when the logic changes. Consider extracting the retain logic into a shared helper (e.g. fn retain_referenced_private_members(body: &mut Vec<ClassMember>)) used by both visitors.
MemberAuthor
There was a problem hiding this comment.
Fixed. Extracted retain_referenced_private_members(body: &mut Vec<ClassMember>) on ClassMemberRefCollector, called from both visit_mut_class_decl and visit_mut_class_expr.
- Namespace JS native private names with # prefix to avoid collisions with TS private members of the same name - Track TS private member accesses on non-this receivers (e.g. a.x in static methods) by maintaining a set of known TS-private names - Use visit_children_with for full traversal including computed member expressions - Extract retain logic into shared retain_referenced_private_members() helper used by both visit_mut_class_decl and visit_mut_class_expr
VaguelySerious
left a comment
Member
There was a problem hiding this comment.
AI review: no blocking issues
| // accesses like `a.x` / `b.x` are not missed. | ||
| MemberProp::Ident(ident) => { | ||
| let name = ident.sym.to_string(); | ||
| if matches!(&*expr.obj, Expr::This(_)) || self.ts_private_names.contains(&name) { |
There was a problem hiding this comment.
AI Review: Note
For MemberProp::Ident, any this.foo access records foo in referenced, not only TS-private members. Retain still keys off private_member_name, so this is mostly extra churn in the set rather than incorrect retention, but it is slightly less precise than filtering to private names only.
MemberAuthor
There was a problem hiding this comment.
Correct - the collector is conservative: it records all this.foo accesses, not just those matching known private names. This is intentional for simplicity. The extra entries in the referenced set are harmless since retain only removes members that private_member_name() identifies as private. Public members are always kept regardless of what's in the set.
| }); | ||
| ``` | ||
| This optimization is critical for SDK classes like `Run` where private helper methods reference Node.js-only imports (encryption, world access, etc.) — eliminating them allows the downstream module-level DCE to also remove those imports from the workflow bundle. |
There was a problem hiding this comment.
AI Review: Nit
This section explains the algorithm well. If you want to reduce future issue reports, a short explicit note on unsupported patterns (for example computed this[...] access to private fields) could help set expectations.
MemberAuthor
There was a problem hiding this comment.
Good suggestion. Computed access like this[name] where name happens to be a private member name won't be tracked, which could lead to incorrect elimination. In practice this pattern is very rare for private members (especially in SDK code like Run), but worth noting if we want to document limitations.
TooTallNate
enabled auto-merge (squash)
April 9, 2026 19:22
VaguelySerious
approved these changes
Apr 9, 2026
Uh oh!
There was an error while loading. Please reload this page.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
After stripping
"use step"methods from a class body in workflow mode, eliminate private members that are no longer referenced by any remaining public member. This applies to both:#field,#method()private:private field,private method()The algorithm is iterative — references are seeded from public members, then expanded through surviving private members until a fixed point, enabling cascading elimination.
Example
Input:
Workflow mode output:
The cascading elimination is key:
encryptionKeyPromiseis only referenced bygetEncryptionKey(), which is itself only referenced by the strippedvaluegetter andcancel()method. Both private members are removed, allowing the downstream module-level DCE to also eliminate theimportKeyandgetWorldimports.Motivation
SDK classes like
Runhave private helper methods that reference Node.js-only imports (encryption, world access). Without this optimization, those helpers survive into the workflow bundle even though nothing calls them after"use step"bodies are stripped, keeping the Node.js imports alive and preventing tree-shaking.Test
New fixture:
private-member-dce/input.tswith expected outputs for all three modes.