Uh oh!
There was an error while loading. Please reload this page.
fix(swc-plugin): count destructuring-default references in DCE usage analysis - #2398
Merged
Conversation
…analysis
The DCE usage collector skipped the entire variable name pattern when
visiting a `VarDeclarator` (to avoid marking the binding name as "used").
But default-value initializers inside destructuring patterns live in that
pattern — e.g. the `TTL` in `const { ttl = TTL } = options;` — so those
references were invisible to the collector. A module-scope `const`
referenced only through such a default was treated as unused and stripped,
while the surviving code kept reading it, producing a runtime
`ReferenceError` when the default fired.
Traverse the default-value initializer expressions (and computed keys)
within destructuring patterns while still not marking the binding names
themselves, so the referenced declaration is preserved. Function-parameter
defaults were already covered (params are visited in full).
Fixes#2396.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>🦋 Changeset detectedLatest commit: 9006ec3 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
Contributor
📊 Benchmark Results
workflow with no steps💻 Local Development
▲ Production (Vercel)
workflow with 1 step💻 Local Development
▲ Production (Vercel)
workflow with 10 sequential steps💻 Local Development
▲ Production (Vercel)
workflow with 25 sequential steps💻 Local Development
▲ Production (Vercel)
workflow with 50 sequential steps💻 Local Development
▲ Production (Vercel)
Promise.all with 10 concurrent steps💻 Local Development
▲ Production (Vercel)
Promise.all with 25 concurrent steps💻 Local Development
▲ Production (Vercel)
Promise.all with 50 concurrent steps💻 Local Development
▲ Production (Vercel)
Promise.race with 10 concurrent steps💻 Local Development
▲ Production (Vercel)
Promise.race with 25 concurrent steps💻 Local Development
▲ Production (Vercel)
Promise.race with 50 concurrent steps💻 Local Development
▲ Production (Vercel)
workflow with 10 sequential data payload steps (10KB)💻 Local Development
▲ Production (Vercel)
workflow with 25 sequential data payload steps (10KB)💻 Local Development
▲ Production (Vercel)
workflow with 50 sequential data payload steps (10KB)💻 Local Development
▲ Production (Vercel)
workflow with 10 concurrent data payload steps (10KB)💻 Local Development
▲ Production (Vercel)
workflow with 25 concurrent data payload steps (10KB)💻 Local Development
▲ Production (Vercel)
workflow with 50 concurrent data payload steps (10KB)💻 Local Development
▲ Production (Vercel)
Stream Benchmarks(includes TTFB metrics)workflow with stream💻 Local Development
▲ Production (Vercel)
stream pipeline with 5 transform steps (1MB)💻 Local Development
▲ Production (Vercel)
10 parallel streams (1MB each)💻 Local Development
▲ Production (Vercel)
fan-out fan-in 10 streams (1MB each)💻 Local Development
▲ Production (Vercel)
SummaryFastest Framework by WorldWinner determined by most benchmark wins
Fastest World by FrameworkWinner determined by most benchmark wins
Column Definitions
Worlds:
❌ Some benchmark jobs failed:
Check the workflow run for details. |
Contributor
🧪 E2E Test Results✅ All tests passed Summary
Details by Category✅ ▲ Vercel Production
✅ 💻 Local Development
✅ 📦 Local Production
✅ 🐘 Local Postgres
✅ 🪟 Windows
✅ 📋 Other
❌ Some E2E test jobs failed:
Check the workflow run for details. |
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes the SWC plugin’s DCE usage analysis so module-scope declarations referenced only via destructuring default initializers (e.g. const { ttl = TTL } = options) are correctly counted as “used”, preventing runtime ReferenceErrors when defaults fire.
Changes:
- Extend
ComprehensiveUsageCollectorto traverse destructuring patterns and visit only default-value initializers (and computed keys), without marking binding identifiers as used. - Add a regression fixture covering module-scope const preservation in both step and workflow modes (and confirming truly-unused consts are still removed).
- Update the SWC plugin spec and add a changeset for the patch release.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
packages/swc-plugin-workflow/transform/src/lib.rs | Adds visit_pat_default_initializers and wires it into visit_mut_var_declarator so destructuring-default references are counted for DCE. |
packages/swc-plugin-workflow/transform/tests/fixture/destructuring-default-references-module-const/input.js | New fixture input reproducing the destructuring-default-only reference pattern and an unused const. |
packages/swc-plugin-workflow/transform/tests/fixture/destructuring-default-references-module-const/output-step.js | Step-mode snapshot asserting referenced consts survive and unused const is removed. |
packages/swc-plugin-workflow/transform/tests/fixture/destructuring-default-references-module-const/output-workflow.js | Workflow-mode snapshot asserting referenced consts survive and unused const is removed. |
packages/swc-plugin-workflow/spec.md | Documents that destructuring-default initializers count as references for DCE in both modes. |
.changeset/swc-destructuring-default-dce.md | Patch changeset describing the DCE fix and the prevented runtime error. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
* origin/main: docs: document run idempotency (#2011) Render attr_set events and run attributes in observability UI (#2393) [ci] Fix backport job model slug (#2403) [ci] Comment on PR when backport fails, revert to use opus 4.8 (#2400) Update queue client to 0.3.1 (#2399) fix(deps): upgrade esbuild to 0.28.1 (GHSA-gv7w-rqvm-qjhr) (#2395)
TooTallNate
approved these changes
Jun 16, 2026
TooTallNate
left a comment
Member
There was a problem hiding this comment.
Approve — precise root-cause fix, exhaustive pattern coverage, and I reproduced the before/after
Targeted DCE fix: the usage collector deliberately skipped var_decl.name to avoid marking binding names as used, but destructuring-default initializers (const { ttl = TTL } = options;) live inside that pattern, so references like TTL were never counted and DCE stripped the still-referenced declaration → runtime ReferenceError. The diagnosis in the PR matches the code exactly.
What I verified
- The new
visit_pat_default_initializersis exhaustive over the binding-pattern AST and visits only value positions: objectKeyValuevalues + computed keys,Assign.valuedefaults, array element defaults,AssignPat.right, rest args, and expression patterns — while never descending into binding names. I traced each arm against thePat/ObjectPatPropvariants; nothing is missed and the{ ttl = TTL }case correctly skips thettlkey and visits only theTTLdefault. - It cannot over-broaden DCE. The collector's
visit_mut_identblindly adds toused_identifiers, and this helper only ever adds references — the safe direction for DCE (over-counting preserves declarations, never wrongly strips). ThePat::Exprarm visiting a member expression's property ident is the same conservative behavior the collector already has when walking initializers, so it introduces no new risk. - Built and ran the full Rust suite: 128 fixture tests + error tests, all green.
- Reproduced the regression: reverted just the
visit_pat_default_initializerscall site to simulatemain, and both the step-mode and workflow-mode fixtures FAIL — the emitted output stripsconst TTL/const RETRIESwhile keeping the code that references them. Restored the fix → both pass. This is the before/after the PR claims, confirmed locally. - The fixture's negative case earns its keep:
const UNUSED = 'dead'is still stripped in both outputs, proving the fix preserves referenced declarations only, not everything in a pattern's vicinity. - The fix covers the real latent bug, not just the synthetic repro:
workbench/vitest/workflows/cookbook/distributed-abort-controller.ts:126hasconst { ttlMs = DEFAULT_TTL_MS, graceMs = DEFAULT_GRACE_MS } = options;referencing module consts at lines 16/18 — exactly this pattern. The fixture is a faithful minimal reduction of it. - spec.md updated per AGENTS.md — both the step-mode and workflow-mode DCE sections gain the destructuring-default note (correct; spec.md has no version frontmatter, the bump rule applies to
skills/). Changeset is a correctly-scoped@workflow/swc-pluginpatch.
Notes
- The fixture covers a class static method and a plain exported function, deliberately demonstrating the bug is class- and mode-independent (matching the issue). Good instinct to prove both, since the worked-around precedent (#1858 kill-switch) might have suggested it was class-specific.
- Function-parameter destructuring defaults were already handled (
visit_mut_fn_declvisits params in full, per the existingdefault-parameter-usagefixture) — the PR correctly scopes the fix to var declarators only and says so. I'd have flagged it if param defaults regressed, but they're untouched and the existing fixture still passes. - Distinct from #1944 as the PR notes: that was DCE ordering vs step hoisting; this is the collector never traversing these initializers at all. No nested/hoisted steps involved, which the fixture confirms.
CI lanes that are red are the session's known baseline flakes (Windows dev-server / express benchmark), unrelated to a Rust-only change. LGTM.
Uh oh!
There was an error while loading. Please reload this page.
github-actionsBot
added a commit
that referenced
this pull request
Jun 16, 2026
…analysis (#2398) The DCE usage collector skipped the entire variable name pattern when visiting a `VarDeclarator` (to avoid marking the binding name as "used"). But default-value initializers inside destructuring patterns live in that pattern — e.g. the `TTL` in `const { ttl = TTL } = options;` — so those references were invisible to the collector. A module-scope `const` referenced only through such a default was treated as unused and stripped, while the surviving code kept reading it, producing a runtime `ReferenceError` when the default fired. Traverse the default-value initializer expressions (and computed keys) within destructuring patterns while still not marking the binding names themselves, so the referenced declaration is preserved. Function-parameter defaults were already covered (params are visited in full). Fixes#2396. Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Pranay Prakash <pranay.gp@gmail.com>
Contributor
Backport PR opened against |
pranaygp pushed a commit
that referenced
this pull request
Jun 17, 2026
…analysis (#2398) (#2465) The DCE usage collector skipped the entire variable name pattern when visiting a `VarDeclarator` (to avoid marking the binding name as "used"). But default-value initializers inside destructuring patterns live in that pattern — e.g. the `TTL` in `const { ttl = TTL } = options;` — so those references were invisible to the collector. A module-scope `const` referenced only through such a default was treated as unused and stripped, while the surviving code kept reading it, producing a runtime `ReferenceError` when the default fired. Traverse the default-value initializer expressions (and computed keys) within destructuring patterns while still not marking the binding names themselves, so the referenced declaration is preserved. Function-parameter defaults were already covered (params are visited in full). Fixes#2396. Signed-off-by: Pranay Prakash <pranay.gp@gmail.com> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
This was referenced Jun 17, 2026
Merged
TooTallNate added a commit
that referenced
this pull request
Jun 17, 2026
…default DCE test The #2398 backport (#2465) added the destructuring-default-references-module-const fixture but committed output-step.js verbatim from main and omitted output-client.js entirely. Stable's SWC plugin emits a different step-registration shape (registerStepFunction import vs. inline IIFE) and has less aggressive DCE (the truly-unused UNUSED const is not stripped), so the copied output-step.js did not match stable's actual output and the required output-client.js was missing. Regenerate both fixture outputs against stable's plugin so the client_mode and step_mode fixture tests pass. The regression being tested still holds: TTL and RETRIES (referenced only via destructuring defaults) are preserved in all modes. Signed-off-by: Nathan Rajlich <n@n8.io>
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.
Fixes#2396.
Problem
The SWC plugin's dead-code-elimination (DCE) usage analysis did not count identifier references that appear as default values inside a destructuring pattern (e.g.
const { ttl = TTL } = options;). When a module-scopeconstwas referenced only through such a destructuring default, the DCE pass treated it as unused and stripped it from the emitted bundle — while keeping the code that referenced it. The result was a runtimeReferenceError: <const> is not definedwhen the default fired.This is mode-independent (reproduces in both step and workflow mode) and class-independent (a plain top-level function exhibits it too). It shipped as a worked-around bug in the kill-switch pattern (#1858) and remains latent in
workbench/vitest/workflows/cookbook/distributed-abort-controller.ts.Root cause
In
ComprehensiveUsageCollector::visit_mut_var_declarator, the collector visited only the initializer and deliberately skipped the entirevar_decl.namepattern (to avoid marking the binding name as "used"). For destructuring patterns, the default-value initializer expressions live insidevar_decl.name— so references like theTTLin{ ttl = TTL }were never added toused_identifiers, andremove_dead_codethen pruned the still-referenced declaration.Fix
Added
visit_pat_default_initializers, which walks a binding pattern and visits only the default-value initializer expressions (and computed keys) —ObjectPatProp::KeyValuevalues,ObjectPatProp::Assign.value,AssignPat.right, array element defaults, rest args — while still not marking the binding names themselves as used.visit_mut_var_declaratornow calls it onvar_decl.name.Function-parameter destructuring defaults were already covered:
visit_mut_fn_declvisits params in full (confirmed by the existingdefault-parameter-usagefixture), so no change was needed there.Tests
destructuring-default-references-module-const/asserts that two module-scope consts referenced only via destructuring defaults — one inside a class static method, one inside a plain exported function — survive in both step and workflow mode. A third, genuinely-unused const is still stripped, confirming DCE is not over-broadened.main(consts stripped) and passes with the fix.cargo test -p swc_workflowsuite green (128 fixture tests + error tests).Spec
Updated
packages/swc-plugin-workflow/spec.md(both the step-mode and workflow-mode DCE sections) to note that a reference counts even when it appears only inside a destructuring-default initializer.Relation to prior fixes
Same symptom class as #1944 (DCE removing a declaration that surviving code references) but a distinct root cause: #1944 was about DCE ordering relative to step hoisting; this is about the usage collector never traversing destructuring-default initializers at all, and reproduces with no nested/hoisted steps involved.
🤖 Generated with Claude Code