[swc-plugin] Preserve imports referenced by hoisted nested steps#1944
[swc-plugin] Preserve imports referenced by hoisted nested steps#1944TooTallNate merged 3 commits intomainfrom
Conversation
Dead-code elimination ran before nested step functions were hoisted out of workflow bodies, so imports referenced only by hoisted step bodies were incorrectly stripped from the step bundle, causing a ReferenceError at runtime. Move DCE to run after hoisting in visit_mut_program.
🦋 Changeset detectedLatest commit: 9d132e5 The changes in this PR will be included in the next version bump. This PR includes changesets to release 19 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 |
🧪 E2E Test Results✅ All tests passed Summary
Details by Category✅ ▲ Vercel Production
✅ 💻 Local Development
✅ 📦 Local Production
✅ 🐘 Local Postgres
✅ 🪟 Windows
✅ 📋 Other
|
📊 Benchmark Results
workflow with no steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Nitro | Next.js (Turbopack) workflow with 1 step💻 Local Development
▲ Production (Vercel)
🔍 Observability: Nitro | Next.js (Turbopack) workflow with 10 sequential steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Nitro | Next.js (Turbopack) workflow with 25 sequential steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Nitro | Next.js (Turbopack) workflow with 50 sequential steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Nitro | Next.js (Turbopack) Promise.all with 10 concurrent steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Nitro | Next.js (Turbopack) Promise.all with 25 concurrent steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Nitro | Next.js (Turbopack) Promise.all with 50 concurrent steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Nitro | Next.js (Turbopack) Promise.race with 10 concurrent steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Nitro | Next.js (Turbopack) Promise.race with 25 concurrent steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Nitro | Next.js (Turbopack) Promise.race with 50 concurrent steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Next.js (Turbopack) | Nitro workflow with 10 sequential data payload steps (10KB)💻 Local Development
▲ Production (Vercel)
🔍 Observability: Nitro | Next.js (Turbopack) workflow with 25 sequential data payload steps (10KB)💻 Local Development
▲ Production (Vercel)
🔍 Observability: Nitro | Next.js (Turbopack) workflow with 50 sequential data payload steps (10KB)💻 Local Development
▲ Production (Vercel)
🔍 Observability: Nitro | Next.js (Turbopack) workflow with 10 concurrent data payload steps (10KB)💻 Local Development
▲ Production (Vercel)
🔍 Observability: Nitro | Next.js (Turbopack) workflow with 25 concurrent data payload steps (10KB)💻 Local Development
▲ Production (Vercel)
🔍 Observability: Next.js (Turbopack) | Nitro workflow with 50 concurrent data payload steps (10KB)💻 Local Development
▲ Production (Vercel)
🔍 Observability: Next.js (Turbopack) | Nitro Stream Benchmarks (includes TTFB metrics)workflow with stream💻 Local Development
▲ Production (Vercel)
🔍 Observability: Nitro | Next.js (Turbopack) stream pipeline with 5 transform steps (1MB)💻 Local Development
▲ Production (Vercel)
🔍 Observability: Nitro | Next.js (Turbopack) 10 parallel streams (1MB each)💻 Local Development
▲ Production (Vercel)
🔍 Observability: Nitro | Next.js (Turbopack) fan-out fan-in 10 streams (1MB each)💻 Local Development
▲ Production (Vercel)
🔍 Observability: Nitro | Next.js (Turbopack) 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. |
karthikscale3
left a comment
There was a problem hiding this comment.
LGTM. Verified it in the SWC playground.
There was a problem hiding this comment.
Pull request overview
This PR adjusts the SWC workflow transform so dead-code elimination runs after nested step hoisting, with the goal of preserving module imports that hoisted step bodies still reference in step-mode output.
Changes:
- Moves
remove_dead_codefromvisit_mut_module_itemsto the end ofvisit_mut_program. - Updates existing step-mode fixtures whose hoisted steps now correctly retain required imports.
- Adds a regression fixture covering nested steps that reference module-level imports.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
packages/swc-plugin-workflow/transform/tests/fixture/nested-steps-in-object-constructor/output-step.js |
Updates expected step output to retain an import used by a hoisted nested step. |
packages/swc-plugin-workflow/transform/tests/fixture/nested-step-with-closure/output-step.js |
Updates expected step output to retain an import referenced from hoisted closure-based steps. |
packages/swc-plugin-workflow/transform/tests/fixture/nested-step-references-module-import/output-workflow.js |
Adds workflow-mode regression output for the new nested-import fixture. |
packages/swc-plugin-workflow/transform/tests/fixture/nested-step-references-module-import/output-step.js |
Adds step-mode regression output showing preserved imports for a hoisted nested step. |
packages/swc-plugin-workflow/transform/tests/fixture/nested-step-references-module-import/input.js |
Adds the new regression input covering nested step references to module imports. |
packages/swc-plugin-workflow/transform/tests/fixture/factory-with-step-method/output-step.js |
Updates expected step output to retain an import used by a hoisted step method. |
packages/swc-plugin-workflow/transform/src/lib.rs |
Defers dead-code elimination until after nested step hoisting completes. |
.changeset/preserve-imports-used-by-hoisted-steps.md |
Records the patch release note for the SWC plugin fix. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…nctions Anonymous steps nested inside callback properties of a non-exported workflow function were registered with an unnamespaced step ID in step mode while the workflow-mode proxy looked them up under the workflow function name, causing a runtime 'step not found' failure. Set current_workflow_function_name in visit_mut_fn_decl for non-exported workflow functions to match the behavior in visit_mut_export_decl. Also clarify the fixture comment to distinguish step-mode and workflow-mode behavior per reviewer feedback.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…n shapes Extends the previous fix to cover all three non-exported workflow declaration forms (async function decl, const arrow, const fn-expr) by visiting the workflow body with workflow context before replacing it, and corrects the __internal_workflows manifest comment to report the same prefixed step IDs that are registered at runtime and looked up by the workflow-mode WORKFLOW_USE_STEP proxy. Adds a dedicated regression fixture covering all three shapes.
* [swc-plugin] Preserve imports referenced by hoisted nested steps Dead-code elimination ran before nested step functions were hoisted out of workflow bodies, so imports referenced only by hoisted step bodies were incorrectly stripped from the step bundle, causing a ReferenceError at runtime. Move DCE to run after hoisting in visit_mut_program. * [swc-plugin] Namespace nested step IDs under non-exported workflow functions Anonymous steps nested inside callback properties of a non-exported workflow function were registered with an unnamespaced step ID in step mode while the workflow-mode proxy looked them up under the workflow function name, causing a runtime 'step not found' failure. Set current_workflow_function_name in visit_mut_fn_decl for non-exported workflow functions to match the behavior in visit_mut_export_decl. Also clarify the fixture comment to distinguish step-mode and workflow-mode behavior per reviewer feedback. * [swc-plugin] Namespace nested step IDs across all workflow declaration shapes Extends the previous fix to cover all three non-exported workflow declaration forms (async function decl, const arrow, const fn-expr) by visiting the workflow body with workflow context before replacing it, and corrects the __internal_workflows manifest comment to report the same prefixed step IDs that are registered at runtime and looked up by the workflow-mode WORKFLOW_USE_STEP proxy. Adds a dedicated regression fixture covering all three shapes. Signed-off-by: Nathan Rajlich <n@n8.io>
|
Backport PR opened against |
Summary
Fixes a bug in the SWC plugin where step-mode bundles dropped module-level imports that were referenced by step functions hoisted out of workflow bodies, causing a
ReferenceErrorat runtime.Root Cause
Ordering bug in the transform pipeline:
StepTransform::visit_mut_module_itemsruns and, at the end, invokesremove_dead_code, which strips any import not referenced by current top-level code.StepTransform::visit_mut_programthen hoists nested step functions out of workflow bodies onto the module top level (e.g.var _anonymousStep0 = async (input) => db.query(input.query);).db, but the import was already deleted in step 1.This only affected steps nested inside workflow function bodies — top-level steps already worked because their bodies were inlined in
itemsbefore DCE ran. Three pre-existing fixtures (nested-step-with-closure,nested-steps-in-object-constructor,factory-with-step-method) had committed buggy outputs that this change corrects.For example, given:
The previous step-mode output omitted
import { db } from './db';while still emittingvar _anonymousStep0 = async (input) => db.query(input.query);.Fix
In
packages/swc-plugin-workflow/transform/src/lib.rs:remove_dead_codecall from the end ofvisit_mut_module_items.remove_dead_code(&mut module.body)call invisit_mut_program, after nested step hoisting completes (right before metadata-comment insertion).Verification
tests/fixture/nested-step-references-module-import/covering both kept-imports (used by hoisted step) and stripped-imports (truly unused, or only used by replaced workflow body).output-step.jsfiles updated to now correctly include the imports their hoisted steps reference.spec.mdalready documented the desired behavior — no spec change needed; this is purely an implementation conformance fix.