fix(swc-plugin): closure variable detection for new expressions and module-level declarations#1368
Open
TooTallNate wants to merge 9 commits intomainfrom
Open
fix(swc-plugin): closure variable detection for new expressions and module-level declarations#1368TooTallNate wants to merge 9 commits intomainfrom
new expressions and module-level declarations#1368TooTallNate wants to merge 9 commits intomainfrom
Conversation
… module-level declarations Fix two SWC compiler plugin bugs related to closure variable detection: 1. Add Expr::New handling to ClosureVariableCollector so `new Class(...args)` properly captures both the callee and arguments as closure variables. 2. Exclude module-level declarations (functions, variables, classes) from closure variable detection, preventing over-capturing of identifiers that are already available in all bundles. This also allows DCE to properly remove step-only helpers and their imports from the workflow bundle. Fixes #1365
🦋 Changeset detectedLatest commit: 796c53a 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
🧪 E2E Test Results❌ Some tests failed Summary
❌ Failed Tests🌍 Community Worlds (55 failed)mongodb (3 failed):
redis (2 failed):
turso (50 failed):
📋 Other (1 failed)e2e-local-postgres-nest-stable (1 failed):
Details by Category✅ ▲ Vercel Production
✅ 💻 Local Development
✅ 📦 Local Production
✅ 🐘 Local Postgres
✅ 🪟 Windows
❌ 🌍 Community Worlds
❌ 📋 Other
❌ Some E2E test jobs failed:
Check the workflow run for details. |
Contributor
📊 Benchmark Results
workflow with no steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Nitro | Next.js (Turbopack) | Express workflow with 1 step💻 Local Development
▲ Production (Vercel)
🔍 Observability: Express | Next.js (Turbopack) | Nitro workflow with 10 sequential steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Nitro | Express | Next.js (Turbopack) 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: Nitro | Express | Next.js (Turbopack) 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: Express | Next.js (Turbopack) | Nitro Promise.all with 50 concurrent steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Nitro | Express | 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 | Next.js (Turbopack) | Nitro Promise.race with 50 concurrent steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Nitro | Next.js (Turbopack) | Express Stream Benchmarks (includes TTFB metrics)workflow with stream💻 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
Fixes SWC plugin closure-variable analysis so step extraction correctly captures identifiers used in new expressions, and avoids over-capturing module-scope declarations (which can lead to attempting to serialize functions and can block DCE of step-only helpers).
Changes:
- Add
Expr::Newhandling inClosureVariableCollectorto analyze constructor callee + args. - Exclude module-level declarations (in addition to imports) from closure-variable detection by seeding collector locals with
declared_identifiers. - Add a new fixture covering both scenarios and update the plugin spec + changelog entry.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/swc-plugin-workflow/transform/src/lib.rs | Implements Expr::New traversal and passes module-level declared identifiers into closure var collection to prevent over-capture. |
| packages/swc-plugin-workflow/spec.md | Documents the updated closure-variable detection behavior. |
| packages/swc-plugin-workflow/transform/tests/fixture/closure-new-expr-and-module-declarations/input.js | New fixture input reproducing both reported bugs. |
| packages/swc-plugin-workflow/transform/tests/fixture/closure-new-expr-and-module-declarations/output-workflow.js | Expected workflow-mode output validating closure capture and DCE-friendly behavior. |
| packages/swc-plugin-workflow/transform/tests/fixture/closure-new-expr-and-module-declarations/output-step.js | Expected step-mode output validating rehydration + direct access to module-level helpers. |
| packages/swc-plugin-workflow/transform/tests/fixture/closure-new-expr-and-module-declarations/output-client.js | Expected client-mode output validating closure capture in non-step bundle. |
| .changeset/fix-swc-closure-variable-bugs.md | Changeset for patch release of @workflow/swc-plugin. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
…ure variable collector
Expand closure variable detection to cover more AST patterns:
Expressions: Seq (comma), Yield, OptChain, Prop::Shorthand,
computed property keys, Prop::Assign defaults, Class (skip bodies)
Statements: Throw, Try/Catch/Finally, Switch, ForIn, ForOf,
DoWhile, Labeled
Also fix existing Prop::Shorthand bug where object shorthand
properties like { url } were not being collected as closure vars.
Extend test fixture with cases for all newly handled patterns.
Fix spec.md wording per review feedback.
…od bodies The closure variable collector was skipping nested function expressions, arrow functions, and method bodies entirely. This meant closure variables used deep inside inner functions (e.g., a variable used inside a ReadableStream's start() method) were not captured. Now the collector walks into nested function/arrow/method/getter/setter bodies while adding their parameters to the local var set, so only truly free variables from the outer step scope are captured. Also add ReadableStream, WritableStream, TransformStream, and other common Web API globals to the known globals list.
…es in closure detection After comparing with Next.js's SWC plugin closure detection approach, identified and fixed remaining gaps: - TypeScript expression wrappers (as, satisfies, !, type assertions, const assertions, instantiation expressions) now traverse to the inner expression instead of being silently skipped - Class expressions and declarations now walk their body members (methods, properties, constructors, static blocks) to detect closure variables used inside them - Document all remaining safe-to-skip Expr variants (This, Lit, SuperProp, MetaProp, PrivateName, Invalid, JSX)
…ion wrappers Add a proper input.ts fixture that tests closure variable detection through real TypeScript syntax: `as`, `satisfies`, `!` (non-null), angle-bracket type assertions, `as const`, and generic function calls. Update test harness to support input.ts files by adding swc_ecma_parser dev-dependency and auto-detecting TypeScript syntax from file extension. Remove the incorrectly placed TypeScript-related test cases from the JS fixture (they were using plain JS syntax, not actual TS wrappers).
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 free
to 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
Fixes all four SWC compiler plugin bugs from #1365 related to closure variables:
newexpressions (e.g.new MockLanguageModelV3(...args)) were not analyzed for closure variables becauseClosureVariableCollector::collect_from_exprhad noExpr::Newcase. Arguments and callee identifiers were silently missed.__private_getClosureVars()has no values outside workflow context. Now the original function body is preserved inline (directive stripped) so JavaScript's normal closure semantics work for direct calls.ReadableStream'sstart()method) were not detected because the collector skipped nested function/arrow/method bodies entirely. Now it walks into them while properly tracking inner parameters as local vars.Additionally, the closure variable collector was expanded to handle many more AST patterns that were previously falling through silently (
Prop::Shorthand,OptChain,Seq,Yield,Throw,Try/Catch/Finally,Switch,ForIn,ForOf,DoWhile,Labeled, computed property keys). Common Web API globals (ReadableStream,WritableStream,TransformStream, etc.) were added to the known globals list.Changes
packages/swc-plugin-workflow/transform/src/lib.rs:Expr::Newarm to closure variable collectordeclared_identifierstocollect_from_function/collect_from_arrow_exprso module-level declarations are excludedProp::Shorthand,OptChain,Seq,Yield, computed keys,Throw,Try,Switch,ForIn,ForOf,DoWhile,LabeledReadableStream,WritableStream,TransformStream, and other Web API globals to known globals listpackages/swc-plugin-workflow/spec.md: Document all new behaviorsclosure-new-expr-and-module-declarationswith 15 test casesAll 172 tests pass with no regressions.
Fixes #1365
Fixes #1369