From 113a671de31238712644a4f5eb68996eba698476 Mon Sep 17 00:00:00 2001 From: Nathan Rajlich Date: Thu, 9 Apr 2026 11:09:37 -0700 Subject: [PATCH 1/3] feat(swc): dead code eliminate unreferenced private class members in 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/private-member-dce.md | 5 + packages/swc-plugin-workflow/spec.md | 54 +++++ .../swc-plugin-workflow/transform/src/lib.rs | 224 +++++++++++++++++- .../tests/fixture/private-member-dce/input.ts | 51 ++++ .../private-member-dce/output-client.js | 54 +++++ .../fixture/private-member-dce/output-step.js | 64 +++++ .../private-member-dce/output-workflow.js | 43 ++++ 7 files changed, 494 insertions(+), 1 deletion(-) create mode 100644 .changeset/private-member-dce.md create mode 100644 packages/swc-plugin-workflow/transform/tests/fixture/private-member-dce/input.ts create mode 100644 packages/swc-plugin-workflow/transform/tests/fixture/private-member-dce/output-client.js create mode 100644 packages/swc-plugin-workflow/transform/tests/fixture/private-member-dce/output-step.js create mode 100644 packages/swc-plugin-workflow/transform/tests/fixture/private-member-dce/output-workflow.js diff --git a/.changeset/private-member-dce.md b/.changeset/private-member-dce.md new file mode 100644 index 0000000000..ef263b713d --- /dev/null +++ b/.changeset/private-member-dce.md @@ -0,0 +1,5 @@ +--- +"@workflow/swc-plugin": patch +--- + +Eliminate unreferenced private class members in workflow mode after `"use step"` stripping diff --git a/packages/swc-plugin-workflow/spec.md b/packages/swc-plugin-workflow/spec.md index 34f204eb1d..b703b7f591 100644 --- a/packages/swc-plugin-workflow/spec.md +++ b/packages/swc-plugin-workflow/spec.md @@ -1164,6 +1164,60 @@ const obj = { **Client mode**: Same as step mode — the getter body is hoisted for `stepId` assignment, original getter preserved. +### Private member dead code elimination + +In workflow mode, after stripping `"use step"` methods and getters from a class body, the plugin eliminates private class members that are no longer referenced by any remaining (non-private) member. This applies to both: + +- **JS native private members**: `#field`, `#method()` (`ClassMember::PrivateMethod`, `ClassMember::PrivateProp`) +- **TypeScript `private` members**: `private field`, `private method()` (`ClassMethod`/`ClassProp` with `accessibility: Private`) + +The algorithm is iterative: references are first collected from all public members, then the referenced set is expanded by scanning surviving private members' bodies for cross-references, repeating until the set stabilizes. This enables cascading elimination — a private field only referenced by a private method that is itself unreferenced will also be removed. + +Input: +```typescript +export class Run { + static [WORKFLOW_SERIALIZE](instance) { return { id: instance.id }; } + static [WORKFLOW_DESERIALIZE](data) { return new Run(data.id); } + + id: string; + private encryptionKeyPromise: Promise | null = null; + + private async getEncryptionKey() { + if (!this.encryptionKeyPromise) { + this.encryptionKeyPromise = importKey(this.id); + } + return this.encryptionKeyPromise; + } + + constructor(id: string) { this.id = id; } + + get value(): Promise { + 'use step'; + return this.getEncryptionKey().then(() => getWorld().get(this.id)); + } +} +``` + +Workflow output: +```javascript +export class Run { + static [WORKFLOW_SERIALIZE](instance) { return { id: instance.id }; } + static [WORKFLOW_DESERIALIZE](data) { return new Run(data.id); } + id; + // private encryptionKeyPromise — ELIMINATED (only referenced by getEncryptionKey) + // private getEncryptionKey() — ELIMINATED (only referenced by stripped getter) + constructor(id) { this.id = id; } +} +// getter replaced with step proxy +var __step_Run$value = globalThis[Symbol.for("WORKFLOW_USE_STEP")]("step_id"); +Object.defineProperty(Run.prototype, "value", { + get() { return __step_Run$value.call(this); }, + configurable: true, enumerable: false +}); +``` + +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. + --- ## Parameter Handling diff --git a/packages/swc-plugin-workflow/transform/src/lib.rs b/packages/swc-plugin-workflow/transform/src/lib.rs index c1652ad98d..23c8ea21c4 100644 --- a/packages/swc-plugin-workflow/transform/src/lib.rs +++ b/packages/swc-plugin-workflow/transform/src/lib.rs @@ -6,7 +6,7 @@ use swc_core::{ common::{errors::HANDLER, SyntaxContext, DUMMY_SP}, ecma::{ ast::*, - visit::{noop_visit_mut_type, VisitMut, VisitMutWith}, + visit::{noop_visit_mut_type, noop_visit_type, Visit, VisitMut, VisitMutWith, VisitWith}, }, }; @@ -491,6 +491,146 @@ impl TryFrom<&Expr> for Name { } } +/// Collects all member names referenced within an AST subtree via +/// `this.foo` or `this.#foo` patterns. Used after stripping `"use step"` +/// methods in workflow mode to determine which private class members are +/// still referenced by the remaining body, so unreferenced ones can be +/// dead-code-eliminated. +/// +/// Handles both: +/// - JS native private members (`#field`, `#method()`) — detected via +/// `MemberProp::PrivateName` +/// - TypeScript `private` members (`private field`, `private method()`) — +/// detected via `MemberProp::Ident` on `this` expressions +struct ClassMemberRefCollector { + /// All member names referenced via `this.name` or `this.#name` + referenced: HashSet, +} + +impl ClassMemberRefCollector { + fn new() -> Self { + Self { + referenced: HashSet::new(), + } + } + + /// Collects all member names transitively referenced by non-private + /// (public) members of the class. Private members that are only + /// referenced by other private members (which are themselves + /// unreferenced) are NOT included, enabling cascading elimination. + /// + /// Algorithm: seed the referenced set from public members, then + /// iteratively expand by adding references from surviving private + /// members until the set stabilizes. + fn collect_from_class_body(body: &[ClassMember]) -> HashSet { + // Phase 1: collect references from all non-private members + let mut collector = Self::new(); + for member in body { + if !Self::is_private_member(member) { + member.visit_with(&mut collector); + } + } + + // Phase 2: iteratively expand — if a private member is referenced, + // its body may reference other private members + loop { + let prev_len = collector.referenced.len(); + for member in body { + if let Some(name) = Self::private_member_name(member) { + if collector.referenced.contains(&name) { + // This private member survived; scan its body for + // references to other private members + match member { + ClassMember::PrivateMethod(m) => { + if let Some(body) = &m.function.body { + body.visit_with(&mut collector); + } + } + ClassMember::PrivateProp(p) => { + if let Some(value) = &p.value { + value.visit_with(&mut collector); + } + } + ClassMember::Method(m) => { + if let Some(body) = &m.function.body { + body.visit_with(&mut collector); + } + } + ClassMember::ClassProp(p) => { + if let Some(value) = &p.value { + value.visit_with(&mut collector); + } + } + _ => {} + } + } + } + } + if collector.referenced.len() == prev_len { + break; // fixed point reached + } + } + + collector.referenced + } + + /// Returns true if the member is a private member (JS native or TS). + fn is_private_member(member: &ClassMember) -> bool { + matches!( + member, + ClassMember::PrivateMethod(_) | ClassMember::PrivateProp(_) + ) || matches!(member, ClassMember::Method(m) if m.accessibility == Some(Accessibility::Private)) + || matches!(member, ClassMember::ClassProp(p) if p.accessibility == Some(Accessibility::Private)) + } + + /// Returns the name of a private member, if applicable. + fn private_member_name(member: &ClassMember) -> Option { + match member { + ClassMember::PrivateMethod(m) => Some(m.key.name.to_string()), + ClassMember::PrivateProp(p) => Some(p.key.name.to_string()), + ClassMember::Method(m) if m.accessibility == Some(Accessibility::Private) => { + match &m.key { + PropName::Ident(i) => Some(i.sym.to_string()), + PropName::Str(s) => Some(s.value.to_string_lossy().to_string()), + _ => None, + } + } + ClassMember::ClassProp(p) if p.accessibility == Some(Accessibility::Private) => { + match &p.key { + PropName::Ident(i) => Some(i.sym.to_string()), + PropName::Str(s) => Some(s.value.to_string_lossy().to_string()), + _ => None, + } + } + _ => None, + } + } +} + +impl Visit for ClassMemberRefCollector { + noop_visit_type!(); + + fn visit_member_expr(&mut self, expr: &MemberExpr) { + 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()); + } + } + _ => {} + } + // Continue visiting children + expr.obj.visit_with(self); + } +} + // Visitor to collect closure variables from a nested step function struct ClosureVariableCollector { closure_vars: HashSet, @@ -8194,6 +8334,54 @@ impl VisitMut for StepTransform { } true }); + + // After stripping "use step" methods, eliminate private class + // members (both JS native `#field`/`#method()` and TypeScript + // `private field`/`private method()`) that are no longer + // referenced by any remaining member. This prevents dead + // private helpers from keeping Node.js imports alive in the + // workflow bundle. + let referenced = + ClassMemberRefCollector::collect_from_class_body(&class_decl.class.body); + class_decl.class.body.retain(|member| { + match member { + // JS native private: #method() + ClassMember::PrivateMethod(m) => { + referenced.contains(&m.key.name.to_string()) + } + // JS native private: #field + ClassMember::PrivateProp(p) => referenced.contains(&p.key.name.to_string()), + // TS private: private method() + ClassMember::Method(m) + if m.accessibility == Some(Accessibility::Private) => + { + if let Some(name) = match &m.key { + PropName::Ident(i) => Some(i.sym.to_string()), + PropName::Str(s) => Some(s.value.to_string_lossy().to_string()), + _ => None, + } { + referenced.contains(&name) + } else { + true // keep if we can't determine the name + } + } + // TS private: private field + ClassMember::ClassProp(p) + if p.accessibility == Some(Accessibility::Private) => + { + if let Some(name) = match &p.key { + PropName::Ident(i) => Some(i.sym.to_string()), + PropName::Str(s) => Some(s.value.to_string_lossy().to_string()), + _ => None, + } { + referenced.contains(&name) + } else { + true + } + } + _ => true, + } + }); } } @@ -8312,6 +8500,40 @@ impl VisitMut for StepTransform { } true }); + + // 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()), + ClassMember::Method(m) if m.accessibility == Some(Accessibility::Private) => { + if let Some(name) = match &m.key { + PropName::Ident(i) => Some(i.sym.to_string()), + PropName::Str(s) => Some(s.value.to_string_lossy().to_string()), + _ => None, + } { + referenced.contains(&name) + } else { + true + } + } + ClassMember::ClassProp(p) + if p.accessibility == Some(Accessibility::Private) => + { + if let Some(name) = match &p.key { + PropName::Ident(i) => Some(i.sym.to_string()), + PropName::Str(s) => Some(s.value.to_string_lossy().to_string()), + _ => None, + } { + referenced.contains(&name) + } else { + true + } + } + _ => true, + }); } } diff --git a/packages/swc-plugin-workflow/transform/tests/fixture/private-member-dce/input.ts b/packages/swc-plugin-workflow/transform/tests/fixture/private-member-dce/input.ts new file mode 100644 index 0000000000..05c836a0f8 --- /dev/null +++ b/packages/swc-plugin-workflow/transform/tests/fixture/private-member-dce/input.ts @@ -0,0 +1,51 @@ +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 | null = null; + + // TS private method — only called by stripped getters/methods + private async getEncryptionKey(): Promise { + if (!this.encryptionKeyPromise) { + this.encryptionKeyPromise = importKey(this.id); + } + return this.encryptionKeyPromise; + } + + // Public field — should always be kept + public name: string = ''; + + constructor(id: string) { + this.id = id; + } + + // Step getter — references private members, will be stripped + get value(): Promise { + 'use step'; + return this.getEncryptionKey().then(() => getWorld().get(this.id)); + } + + // Step method — references private members, will be stripped + async cancel(): Promise { + 'use step'; + const key = await this.getEncryptionKey(); + await getWorld().cancel(this.id, key); + } + + // Non-step public method — should be kept + toString(): string { + return `Run(${this.id})`; + } +} diff --git a/packages/swc-plugin-workflow/transform/tests/fixture/private-member-dce/output-client.js b/packages/swc-plugin-workflow/transform/tests/fixture/private-member-dce/output-client.js new file mode 100644 index 0000000000..e16a7fcdfa --- /dev/null +++ b/packages/swc-plugin-workflow/transform/tests/fixture/private-member-dce/output-client.js @@ -0,0 +1,54 @@ +import { WORKFLOW_SERIALIZE, WORKFLOW_DESERIALIZE } from '@workflow/serde'; +import { getWorld } from './world.js'; +import { importKey } from './encryption.js'; +/**__internal_workflows{"steps":{"input.ts":{"Run#cancel":{"stepId":"step//./input//Run#cancel"},"Run#value":{"stepId":"step//./input//Run#value"}}},"classes":{"input.ts":{"Run":{"classId":"class//./input//Run"}}}}*/; +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 | null = null; + // TS private method — only called by stripped getters/methods + private async getEncryptionKey(): Promise { + if (!this.encryptionKeyPromise) { + this.encryptionKeyPromise = importKey(this.id); + } + return this.encryptionKeyPromise; + } + // Public field — should always be kept + public name: string = ''; + constructor(id: string){ + this.id = id; + } + // Step getter — references private members, will be stripped + get value(): Promise { + return this.getEncryptionKey().then(()=>getWorld().get(this.id)); + } + // Step method — references private members, will be stripped + async cancel(): Promise { + const key = await this.getEncryptionKey(); + await getWorld().cancel(this.id, key); + } + // Non-step public method — should be kept + toString(): string { + return `Run(${this.id})`; + } +} +(function(__wf_cls, __wf_id) { + var __wf_sym = Symbol.for("workflow-class-registry"), __wf_reg = globalThis[__wf_sym] || (globalThis[__wf_sym] = new Map()); + __wf_reg.set(__wf_id, __wf_cls); + Object.defineProperty(__wf_cls, "classId", { + value: __wf_id, + writable: false, + enumerable: false, + configurable: false + }); +})(Run, "class//./input//Run"); diff --git a/packages/swc-plugin-workflow/transform/tests/fixture/private-member-dce/output-step.js b/packages/swc-plugin-workflow/transform/tests/fixture/private-member-dce/output-step.js new file mode 100644 index 0000000000..df3453214a --- /dev/null +++ b/packages/swc-plugin-workflow/transform/tests/fixture/private-member-dce/output-step.js @@ -0,0 +1,64 @@ +import { WORKFLOW_SERIALIZE, WORKFLOW_DESERIALIZE } from '@workflow/serde'; +import { getWorld } from './world.js'; +import { importKey } from './encryption.js'; +/**__internal_workflows{"steps":{"input.ts":{"Run#cancel":{"stepId":"step//./input//Run#cancel"},"Run#value":{"stepId":"step//./input//Run#value"}}},"classes":{"input.ts":{"Run":{"classId":"class//./input//Run"}}}}*/; +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 | null = null; + // TS private method — only called by stripped getters/methods + private async getEncryptionKey(): Promise { + if (!this.encryptionKeyPromise) { + this.encryptionKeyPromise = importKey(this.id); + } + return this.encryptionKeyPromise; + } + // Public field — should always be kept + public name: string = ''; + constructor(id: string){ + this.id = id; + } + // Step getter — references private members, will be stripped + get value(): Promise { + return this.getEncryptionKey().then(()=>getWorld().get(this.id)); + } + // Step method — references private members, will be stripped + async cancel(): Promise { + const key = await this.getEncryptionKey(); + await getWorld().cancel(this.id, key); + } + // Non-step public method — should be kept + toString(): string { + return `Run(${this.id})`; + } +} +(function(__wf_fn, __wf_id) { + var __wf_sym = Symbol.for("@workflow/core//registeredSteps"), __wf_reg = globalThis[__wf_sym] || (globalThis[__wf_sym] = new Map()); + __wf_reg.set(__wf_id, __wf_fn); + __wf_fn.stepId = __wf_id; +})(Run.prototype["cancel"], "step//./input//Run#cancel"); +(function(__wf_fn, __wf_id) { + var __wf_sym = Symbol.for("@workflow/core//registeredSteps"), __wf_reg = globalThis[__wf_sym] || (globalThis[__wf_sym] = new Map()); + __wf_reg.set(__wf_id, __wf_fn); + __wf_fn.stepId = __wf_id; +})(Object.getOwnPropertyDescriptor(Run.prototype, "value").get, "step//./input//Run#value"); +(function(__wf_cls, __wf_id) { + var __wf_sym = Symbol.for("workflow-class-registry"), __wf_reg = globalThis[__wf_sym] || (globalThis[__wf_sym] = new Map()); + __wf_reg.set(__wf_id, __wf_cls); + Object.defineProperty(__wf_cls, "classId", { + value: __wf_id, + writable: false, + enumerable: false, + configurable: false + }); +})(Run, "class//./input//Run"); diff --git a/packages/swc-plugin-workflow/transform/tests/fixture/private-member-dce/output-workflow.js b/packages/swc-plugin-workflow/transform/tests/fixture/private-member-dce/output-workflow.js new file mode 100644 index 0000000000..0955858d1d --- /dev/null +++ b/packages/swc-plugin-workflow/transform/tests/fixture/private-member-dce/output-workflow.js @@ -0,0 +1,43 @@ +import { WORKFLOW_SERIALIZE, WORKFLOW_DESERIALIZE } from '@workflow/serde'; +/**__internal_workflows{"steps":{"input.ts":{"Run#cancel":{"stepId":"step//./input//Run#cancel"},"Run#value":{"stepId":"step//./input//Run#value"}}},"classes":{"input.ts":{"Run":{"classId":"class//./input//Run"}}}}*/; +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; + // Public field — should always be kept + public name: string = ''; + constructor(id: string){ + this.id = id; + } + // Non-step public method — should be kept + toString(): string { + return `Run(${this.id})`; + } +} +Run.prototype["cancel"] = globalThis[Symbol.for("WORKFLOW_USE_STEP")]("step//./input//Run#cancel"); +var __step_Run$value = globalThis[Symbol.for("WORKFLOW_USE_STEP")]("step//./input//Run#value"); +Object.defineProperty(Run.prototype, "value", { + get () { + return __step_Run$value.call(this); + }, + configurable: true, + enumerable: false +}); +(function(__wf_cls, __wf_id) { + var __wf_sym = Symbol.for("workflow-class-registry"), __wf_reg = globalThis[__wf_sym] || (globalThis[__wf_sym] = new Map()); + __wf_reg.set(__wf_id, __wf_cls); + Object.defineProperty(__wf_cls, "classId", { + value: __wf_id, + writable: false, + enumerable: false, + configurable: false + }); +})(Run, "class//./input//Run"); From 62c755ab7257b43ae16c24b387d7af71e40839c4 Mon Sep 17 00:00:00 2001 From: Nathan Rajlich Date: Thu, 9 Apr 2026 11:20:33 -0700 Subject: [PATCH 2/3] test(swc): add fixture for JS native private member DCE (#field, #method) --- .../private-member-dce-native/input.js | 49 +++++++++++++++ .../output-client.js | 50 ++++++++++++++++ .../private-member-dce-native/output-step.js | 60 +++++++++++++++++++ .../output-workflow.js | 41 +++++++++++++ 4 files changed, 200 insertions(+) create mode 100644 packages/swc-plugin-workflow/transform/tests/fixture/private-member-dce-native/input.js create mode 100644 packages/swc-plugin-workflow/transform/tests/fixture/private-member-dce-native/output-client.js create mode 100644 packages/swc-plugin-workflow/transform/tests/fixture/private-member-dce-native/output-step.js create mode 100644 packages/swc-plugin-workflow/transform/tests/fixture/private-member-dce-native/output-workflow.js diff --git a/packages/swc-plugin-workflow/transform/tests/fixture/private-member-dce-native/input.js b/packages/swc-plugin-workflow/transform/tests/fixture/private-member-dce-native/input.js new file mode 100644 index 0000000000..4a88bc33bc --- /dev/null +++ b/packages/swc-plugin-workflow/transform/tests/fixture/private-member-dce-native/input.js @@ -0,0 +1,49 @@ +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) { + return { id: instance.id }; + } + + static [WORKFLOW_DESERIALIZE](data) { + return new Run(data.id); + } + + // Public field — should be kept + id; + + // Native private field — only referenced by #getEncryptionKey + #encryptionKeyPromise = null; + + // Native private field — referenced by toString (public), should survive + #label = 'run'; + + // Native private method — only called by stripped step methods + async #getEncryptionKey() { + if (!this.#encryptionKeyPromise) { + this.#encryptionKeyPromise = importKey(this.id); + } + return this.#encryptionKeyPromise; + } + + constructor(id) { + this.id = id; + } + + get value() { + 'use step'; + return this.#getEncryptionKey().then(() => getWorld().get(this.id)); + } + + async cancel() { + 'use step'; + const key = await this.#getEncryptionKey(); + await getWorld().cancel(this.id, key); + } + + toString() { + return `Run(${this.id}, ${this.#label})`; + } +} diff --git a/packages/swc-plugin-workflow/transform/tests/fixture/private-member-dce-native/output-client.js b/packages/swc-plugin-workflow/transform/tests/fixture/private-member-dce-native/output-client.js new file mode 100644 index 0000000000..5a36143771 --- /dev/null +++ b/packages/swc-plugin-workflow/transform/tests/fixture/private-member-dce-native/output-client.js @@ -0,0 +1,50 @@ +import { WORKFLOW_SERIALIZE, WORKFLOW_DESERIALIZE } from '@workflow/serde'; +import { getWorld } from './world.js'; +import { importKey } from './encryption.js'; +/**__internal_workflows{"steps":{"input.js":{"Run#cancel":{"stepId":"step//./input//Run#cancel"},"Run#value":{"stepId":"step//./input//Run#value"}}},"classes":{"input.js":{"Run":{"classId":"class//./input//Run"}}}}*/; +export class Run { + static [WORKFLOW_SERIALIZE](instance) { + return { + id: instance.id + }; + } + static [WORKFLOW_DESERIALIZE](data) { + return new Run(data.id); + } + // Public field — should be kept + id; + // Native private field — only referenced by #getEncryptionKey + #encryptionKeyPromise = null; + // Native private field — referenced by toString (public), should survive + #label = 'run'; + // Native private method — only called by stripped step methods + async #getEncryptionKey() { + if (!this.#encryptionKeyPromise) { + this.#encryptionKeyPromise = importKey(this.id); + } + return this.#encryptionKeyPromise; + } + constructor(id){ + this.id = id; + } + get value() { + return this.#getEncryptionKey().then(()=>getWorld().get(this.id)); + } + async cancel() { + const key = await this.#getEncryptionKey(); + await getWorld().cancel(this.id, key); + } + toString() { + return `Run(${this.id}, ${this.#label})`; + } +} +(function(__wf_cls, __wf_id) { + var __wf_sym = Symbol.for("workflow-class-registry"), __wf_reg = globalThis[__wf_sym] || (globalThis[__wf_sym] = new Map()); + __wf_reg.set(__wf_id, __wf_cls); + Object.defineProperty(__wf_cls, "classId", { + value: __wf_id, + writable: false, + enumerable: false, + configurable: false + }); +})(Run, "class//./input//Run"); diff --git a/packages/swc-plugin-workflow/transform/tests/fixture/private-member-dce-native/output-step.js b/packages/swc-plugin-workflow/transform/tests/fixture/private-member-dce-native/output-step.js new file mode 100644 index 0000000000..d1beb20982 --- /dev/null +++ b/packages/swc-plugin-workflow/transform/tests/fixture/private-member-dce-native/output-step.js @@ -0,0 +1,60 @@ +import { WORKFLOW_SERIALIZE, WORKFLOW_DESERIALIZE } from '@workflow/serde'; +import { getWorld } from './world.js'; +import { importKey } from './encryption.js'; +/**__internal_workflows{"steps":{"input.js":{"Run#cancel":{"stepId":"step//./input//Run#cancel"},"Run#value":{"stepId":"step//./input//Run#value"}}},"classes":{"input.js":{"Run":{"classId":"class//./input//Run"}}}}*/; +export class Run { + static [WORKFLOW_SERIALIZE](instance) { + return { + id: instance.id + }; + } + static [WORKFLOW_DESERIALIZE](data) { + return new Run(data.id); + } + // Public field — should be kept + id; + // Native private field — only referenced by #getEncryptionKey + #encryptionKeyPromise = null; + // Native private field — referenced by toString (public), should survive + #label = 'run'; + // Native private method — only called by stripped step methods + async #getEncryptionKey() { + if (!this.#encryptionKeyPromise) { + this.#encryptionKeyPromise = importKey(this.id); + } + return this.#encryptionKeyPromise; + } + constructor(id){ + this.id = id; + } + get value() { + return this.#getEncryptionKey().then(()=>getWorld().get(this.id)); + } + async cancel() { + const key = await this.#getEncryptionKey(); + await getWorld().cancel(this.id, key); + } + toString() { + return `Run(${this.id}, ${this.#label})`; + } +} +(function(__wf_fn, __wf_id) { + var __wf_sym = Symbol.for("@workflow/core//registeredSteps"), __wf_reg = globalThis[__wf_sym] || (globalThis[__wf_sym] = new Map()); + __wf_reg.set(__wf_id, __wf_fn); + __wf_fn.stepId = __wf_id; +})(Run.prototype["cancel"], "step//./input//Run#cancel"); +(function(__wf_fn, __wf_id) { + var __wf_sym = Symbol.for("@workflow/core//registeredSteps"), __wf_reg = globalThis[__wf_sym] || (globalThis[__wf_sym] = new Map()); + __wf_reg.set(__wf_id, __wf_fn); + __wf_fn.stepId = __wf_id; +})(Object.getOwnPropertyDescriptor(Run.prototype, "value").get, "step//./input//Run#value"); +(function(__wf_cls, __wf_id) { + var __wf_sym = Symbol.for("workflow-class-registry"), __wf_reg = globalThis[__wf_sym] || (globalThis[__wf_sym] = new Map()); + __wf_reg.set(__wf_id, __wf_cls); + Object.defineProperty(__wf_cls, "classId", { + value: __wf_id, + writable: false, + enumerable: false, + configurable: false + }); +})(Run, "class//./input//Run"); diff --git a/packages/swc-plugin-workflow/transform/tests/fixture/private-member-dce-native/output-workflow.js b/packages/swc-plugin-workflow/transform/tests/fixture/private-member-dce-native/output-workflow.js new file mode 100644 index 0000000000..3d10665f47 --- /dev/null +++ b/packages/swc-plugin-workflow/transform/tests/fixture/private-member-dce-native/output-workflow.js @@ -0,0 +1,41 @@ +import { WORKFLOW_SERIALIZE, WORKFLOW_DESERIALIZE } from '@workflow/serde'; +/**__internal_workflows{"steps":{"input.js":{"Run#cancel":{"stepId":"step//./input//Run#cancel"},"Run#value":{"stepId":"step//./input//Run#value"}}},"classes":{"input.js":{"Run":{"classId":"class//./input//Run"}}}}*/; +export class Run { + static [WORKFLOW_SERIALIZE](instance) { + return { + id: instance.id + }; + } + static [WORKFLOW_DESERIALIZE](data) { + return new Run(data.id); + } + // Public field — should be kept + id; + // Native private field — referenced by toString (public), should survive + #label = 'run'; + constructor(id){ + this.id = id; + } + toString() { + return `Run(${this.id}, ${this.#label})`; + } +} +Run.prototype["cancel"] = globalThis[Symbol.for("WORKFLOW_USE_STEP")]("step//./input//Run#cancel"); +var __step_Run$value = globalThis[Symbol.for("WORKFLOW_USE_STEP")]("step//./input//Run#value"); +Object.defineProperty(Run.prototype, "value", { + get () { + return __step_Run$value.call(this); + }, + configurable: true, + enumerable: false +}); +(function(__wf_cls, __wf_id) { + var __wf_sym = Symbol.for("workflow-class-registry"), __wf_reg = globalThis[__wf_sym] || (globalThis[__wf_sym] = new Map()); + __wf_reg.set(__wf_id, __wf_cls); + Object.defineProperty(__wf_cls, "classId", { + value: __wf_id, + writable: false, + enumerable: false, + configurable: false + }); +})(Run, "class//./input//Run"); From 4ea576a47ee04f7117a81056a307c7013cdb5092 Mon Sep 17 00:00:00 2001 From: Nathan Rajlich Date: Thu, 9 Apr 2026 11:26:53 -0700 Subject: [PATCH 3/3] fix(swc): address review feedback on private member DCE - 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 --- .../swc-plugin-workflow/transform/src/lib.rs | 225 ++++++++---------- 1 file changed, 104 insertions(+), 121 deletions(-) diff --git a/packages/swc-plugin-workflow/transform/src/lib.rs b/packages/swc-plugin-workflow/transform/src/lib.rs index 23c8ea21c4..be3bc688a7 100644 --- a/packages/swc-plugin-workflow/transform/src/lib.rs +++ b/packages/swc-plugin-workflow/transform/src/lib.rs @@ -492,25 +492,33 @@ impl TryFrom<&Expr> for Name { } /// Collects all member names referenced within an AST subtree via -/// `this.foo` or `this.#foo` patterns. Used after stripping `"use step"` -/// methods in workflow mode to determine which private class members are -/// still referenced by the remaining body, so unreferenced ones can be +/// `this.foo`, `this.#foo`, or `obj.foo` (when `foo` is a known +/// TS-private name) patterns. Used after stripping `"use step"` methods +/// in workflow mode to determine which private class members are still +/// referenced by the remaining body, so unreferenced ones can be /// dead-code-eliminated. /// /// Handles both: -/// - JS native private members (`#field`, `#method()`) — detected via -/// `MemberProp::PrivateName` -/// - TypeScript `private` members (`private field`, `private method()`) — -/// detected via `MemberProp::Ident` on `this` expressions +/// - JS native private members (`#field`, `#method()`) — stored with `#` +/// prefix to avoid collisions with TS private members of the same name +/// - TypeScript `private` members — stored without prefix; detected via +/// `this.foo` and also `obj.foo` when `foo` is a known TS-private name +/// (to handle same-class access patterns like `static compare(a, b) { +/// return a.x - b.x }`) struct ClassMemberRefCollector { - /// All member names referenced via `this.name` or `this.#name` + /// All member names referenced. JS native private names are prefixed + /// with `#` (e.g. `"#foo"`), TS private names are unprefixed (`"foo"`). referenced: HashSet, + /// Known TS-private member names in the current class, so that `a.foo` + /// accesses (not just `this.foo`) are recognized as references. + ts_private_names: HashSet, } impl ClassMemberRefCollector { - fn new() -> Self { + fn new(ts_private_names: HashSet) -> Self { Self { referenced: HashSet::new(), + ts_private_names, } } @@ -523,8 +531,30 @@ impl ClassMemberRefCollector { /// iteratively expand by adding references from surviving private /// members until the set stabilizes. fn collect_from_class_body(body: &[ClassMember]) -> HashSet { + // Build the set of known TS-private names for the collector + let ts_private_names: HashSet = body + .iter() + .filter_map(|m| match m { + ClassMember::Method(m) if m.accessibility == Some(Accessibility::Private) => { + match &m.key { + PropName::Ident(i) => Some(i.sym.to_string()), + PropName::Str(s) => Some(s.value.to_string_lossy().to_string()), + _ => None, + } + } + ClassMember::ClassProp(p) if p.accessibility == Some(Accessibility::Private) => { + match &p.key { + PropName::Ident(i) => Some(i.sym.to_string()), + PropName::Str(s) => Some(s.value.to_string_lossy().to_string()), + _ => None, + } + } + _ => None, + }) + .collect(); + // Phase 1: collect references from all non-private members - let mut collector = Self::new(); + let mut collector = Self::new(ts_private_names); for member in body { if !Self::is_private_member(member) { member.visit_with(&mut collector); @@ -540,29 +570,7 @@ impl ClassMemberRefCollector { if collector.referenced.contains(&name) { // This private member survived; scan its body for // references to other private members - match member { - ClassMember::PrivateMethod(m) => { - if let Some(body) = &m.function.body { - body.visit_with(&mut collector); - } - } - ClassMember::PrivateProp(p) => { - if let Some(value) = &p.value { - value.visit_with(&mut collector); - } - } - ClassMember::Method(m) => { - if let Some(body) = &m.function.body { - body.visit_with(&mut collector); - } - } - ClassMember::ClassProp(p) => { - if let Some(value) = &p.value { - value.visit_with(&mut collector); - } - } - _ => {} - } + Self::visit_member_body(member, &mut collector); } } } @@ -574,6 +582,33 @@ impl ClassMemberRefCollector { collector.referenced } + /// Visit the body/initializer of a class member for reference collection. + fn visit_member_body(member: &ClassMember, collector: &mut Self) { + match member { + ClassMember::PrivateMethod(m) => { + if let Some(body) = &m.function.body { + body.visit_with(collector); + } + } + ClassMember::PrivateProp(p) => { + if let Some(value) = &p.value { + value.visit_with(collector); + } + } + ClassMember::Method(m) => { + if let Some(body) = &m.function.body { + body.visit_with(collector); + } + } + ClassMember::ClassProp(p) => { + if let Some(value) = &p.value { + value.visit_with(collector); + } + } + _ => {} + } + } + /// Returns true if the member is a private member (JS native or TS). fn is_private_member(member: &ClassMember) -> bool { matches!( @@ -583,11 +618,13 @@ impl ClassMemberRefCollector { || matches!(member, ClassMember::ClassProp(p) if p.accessibility == Some(Accessibility::Private)) } - /// Returns the name of a private member, if applicable. + /// Returns the canonical name of a private member. JS native private + /// names are prefixed with `#` to avoid collisions with TS private + /// members of the same name. fn private_member_name(member: &ClassMember) -> Option { match member { - ClassMember::PrivateMethod(m) => Some(m.key.name.to_string()), - ClassMember::PrivateProp(p) => Some(p.key.name.to_string()), + ClassMember::PrivateMethod(m) => Some(format!("#{}", m.key.name)), + ClassMember::PrivateProp(p) => Some(format!("#{}", p.key.name)), ClassMember::Method(m) if m.accessibility == Some(Accessibility::Private) => { match &m.key { PropName::Ident(i) => Some(i.sym.to_string()), @@ -605,6 +642,19 @@ impl ClassMemberRefCollector { _ => None, } } + + /// Removes unreferenced private class members from a class body. + /// Call after stripping `"use step"` methods in workflow mode. + fn retain_referenced_private_members(body: &mut Vec) { + let referenced = Self::collect_from_class_body(body); + body.retain(|member| { + if let Some(name) = Self::private_member_name(member) { + referenced.contains(&name) + } else { + true + } + }); + } } impl Visit for ClassMemberRefCollector { @@ -612,22 +662,24 @@ impl Visit for ClassMemberRefCollector { fn visit_member_expr(&mut self, expr: &MemberExpr) { match &expr.prop { - // Native JS private: `this.#foo` + // Native JS private: `this.#foo` — stored as `#foo` MemberProp::PrivateName(name) => { - self.referenced.insert(name.name.to_string()); + self.referenced.insert(format!("#{}", name.name)); } - // 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`) + // TS private or any ident 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) => { - if matches!(&*expr.obj, Expr::This(_)) { - self.referenced.insert(ident.sym.to_string()); + let name = ident.sym.to_string(); + if matches!(&*expr.obj, Expr::This(_)) || self.ts_private_names.contains(&name) { + self.referenced.insert(name); } } _ => {} } - // Continue visiting children - expr.obj.visit_with(self); + // Continue visiting children, including computed property expressions + expr.visit_children_with(self); } } @@ -8338,50 +8390,10 @@ impl VisitMut for StepTransform { // After stripping "use step" methods, eliminate private class // members (both JS native `#field`/`#method()` and TypeScript // `private field`/`private method()`) that are no longer - // referenced by any remaining member. This prevents dead - // private helpers from keeping Node.js imports alive in the - // workflow bundle. - let referenced = - ClassMemberRefCollector::collect_from_class_body(&class_decl.class.body); - class_decl.class.body.retain(|member| { - match member { - // JS native private: #method() - ClassMember::PrivateMethod(m) => { - referenced.contains(&m.key.name.to_string()) - } - // JS native private: #field - ClassMember::PrivateProp(p) => referenced.contains(&p.key.name.to_string()), - // TS private: private method() - ClassMember::Method(m) - if m.accessibility == Some(Accessibility::Private) => - { - if let Some(name) = match &m.key { - PropName::Ident(i) => Some(i.sym.to_string()), - PropName::Str(s) => Some(s.value.to_string_lossy().to_string()), - _ => None, - } { - referenced.contains(&name) - } else { - true // keep if we can't determine the name - } - } - // TS private: private field - ClassMember::ClassProp(p) - if p.accessibility == Some(Accessibility::Private) => - { - if let Some(name) = match &p.key { - PropName::Ident(i) => Some(i.sym.to_string()), - PropName::Str(s) => Some(s.value.to_string_lossy().to_string()), - _ => None, - } { - referenced.contains(&name) - } else { - true - } - } - _ => true, - } - }); + // referenced by any remaining member. + ClassMemberRefCollector::retain_referenced_private_members( + &mut class_decl.class.body, + ); } } @@ -8502,38 +8514,9 @@ impl VisitMut for StepTransform { }); // 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()), - ClassMember::Method(m) if m.accessibility == Some(Accessibility::Private) => { - if let Some(name) = match &m.key { - PropName::Ident(i) => Some(i.sym.to_string()), - PropName::Str(s) => Some(s.value.to_string_lossy().to_string()), - _ => None, - } { - referenced.contains(&name) - } else { - true - } - } - ClassMember::ClassProp(p) - if p.accessibility == Some(Accessibility::Private) => - { - if let Some(name) = match &p.key { - PropName::Ident(i) => Some(i.sym.to_string()), - PropName::Str(s) => Some(s.value.to_string_lossy().to_string()), - _ => None, - } { - referenced.contains(&name) - } else { - true - } - } - _ => true, - }); + ClassMemberRefCollector::retain_referenced_private_members( + &mut class_expr.class.body, + ); } }