fix(builders): bundle aliased project-local helpers instead of externalizing#1885
Conversation
…alizing Externalizing aliased helpers was unsafe: their source on disk could contain further alias imports that Node's ESM loader doesn't know about, causing 'Package subpath ... is not defined by "exports"' / ERR_MODULE_NOT_FOUND at runtime. Now bundle inline so all alias resolution happens at build time.
🦋 Changeset detectedLatest commit: 7a19ee7 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 |
📊 Benchmark Results
workflow with no steps💻 Local Development
workflow with 1 step💻 Local Development
workflow with 10 sequential steps💻 Local Development
workflow with 25 sequential steps💻 Local Development
workflow with 50 sequential steps💻 Local Development
Promise.all with 10 concurrent steps💻 Local Development
Promise.all with 25 concurrent steps💻 Local Development
Promise.all with 50 concurrent steps💻 Local Development
Promise.race with 10 concurrent steps💻 Local Development
Promise.race with 25 concurrent steps💻 Local Development
Promise.race with 50 concurrent steps💻 Local Development
workflow with 10 sequential data payload steps (10KB)💻 Local Development
workflow with 25 sequential data payload steps (10KB)💻 Local Development
workflow with 50 sequential data payload steps (10KB)💻 Local Development
workflow with 10 concurrent data payload steps (10KB)💻 Local Development
workflow with 25 concurrent data payload steps (10KB)💻 Local Development
workflow with 50 concurrent data payload steps (10KB)💻 Local Development
Stream Benchmarks (includes TTFB metrics)workflow with stream💻 Local Development
stream pipeline with 5 transform steps (1MB)💻 Local Development
10 parallel streams (1MB each)💻 Local Development
fan-out fan-in 10 streams (1MB each)💻 Local Development
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. |
🧪 E2E Test Results✅ All tests passed Summary
Details by Category✅ ▲ Vercel Production
✅ 💻 Local Development
✅ 📦 Local Production
✅ 🐘 Local Postgres
✅ 🪟 Windows
✅ 📋 Other
|
There was a problem hiding this comment.
Pull request overview
This PR fixes a runtime regression in @workflow/builders step bundling where project-local helpers imported via tsconfig paths / esbuild alias / self-referencing package names could be externalized to source files that still contain aliased imports, causing Node ESM runtime failures in non-vite-node environments (e.g., Nitro, plain Node ESM).
Changes:
- Update the SWC esbuild plugin resolution behavior to bundle aliased, project-local files inline instead of externalizing them.
- Update/extend builder tests to assert inline bundling for aliased project-local helpers, including a regression test covering the Mux self-referencing package scenario.
- Add a changeset for a patch release of
@workflow/buildersdescribing the fix and rationale.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| packages/builders/src/swc-esbuild-plugin.ts | Treat project-local results from the esbuild alias/paths fallback as bundle-inlined (return null) rather than externalized. |
| packages/builders/src/swc-esbuild-plugin.test.ts | Update the alias test to assert inlining and add a transitive alias regression test (Mux scenario). |
| .changeset/bundle-aliased-project-local-helpers.md | Patch changeset documenting the runtime error fix and behavior change. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
karthikscale3
left a comment
There was a problem hiding this comment.
AI Review
The fix is correct and the right call, but it's worth understanding the subtle trade-off it introduces.
What I like:
The root cause diagnosis is solid. The old behavior was fundamentally broken for a common pattern — self-referencing packages are pretty standard in monorepo setups (Mux, nx, turborepo-style projects all do this). return null to defer back to esbuild is also the idiomatic way to say "let esbuild handle this normally" — it's a clean one-liner fix rather than a complex workaround.
The comment about vi.mock() is also a nice honest admission — the original justification for externalizing in #1613 (so vitest can intercept via vi.mock()) was apparently wrong anyway, since step dependencies can't be mocked regardless of externalization due to architectural constraints. So there's genuinely no regression on the vitest side.
Where I'd push back or want more clarity:
The bundle size trade-off is real and could bite people. If you have a shared helper imported by 10 step files, it now gets inlined into 10 separate bundles instead of being shared as an external module. For a project with many steps and non-trivial helpers, that's a meaningful size increase — and it affects cold start time since each step bundle is larger.
The PR acknowledges this but frames it as "restores pre-#1613 behavior," which is a bit of a hand-wave. Pre-#1613 behavior was arguably also wrong in this regard — this is a good opportunity to note it as a known limitation rather than a resolved state.
What's missing:
There's no attempt at a smarter middle ground: you could theoretically detect whether a project-local aliased helper itself contains alias imports, and only then choose inlining. But honestly that's probably over-engineering it — the simpler blanket rule (all aliased project-local files → inline) is far easier to reason about and maintain, and correctness beats bundle size optimization here.
Bottom line: Merge it. The correctness fix is unambiguous, the regression test directly covers the reported bug, and the bundle size trade-off is acceptable given the alternative is runtime crashes in production.
Summary
Fixes a regression where step files reaching project-local helpers via tsconfig
paths/ esbuild aliases / self-referencing package names crash at runtime in non-vite-node environments (Nitro, plain Node ESM, etc.) with:Package subpath './lib/foo' is not defined by "exports"ERR_MODULE_NOT_FOUNDReported by Mux: muxinc/ai#193.
Root cause
#1613 added an esbuild fallback resolver so aliased imports resolve via tsconfig
paths. When the resolved file is project-local, it gets externalized as a relative path. But the externalized file's source on disk can contain further alias imports (import "@my-pkg/lib/providers"etc.). Node's ESM loader doesn't know about build-time aliases and falls through to the package'sexportsmap — where the subpath isn't defined — and throws.#1644 fixed this for Node builtins (because esbuild marks builtins external). Genuinely project-local files aren't external, so the
!esbuildResult.externalguard doesn't help them.Fix
When the esbuild fallback resolves a bare specifier to a project-local file, bundle it inline instead of externalizing it as a relative path. Bundling resolves all alias imports at build time, so the runtime never sees an unresolvable specifier. Relative-path imports are unaffected and continue to externalize as before — they don't have this problem because Node can resolve relative paths fine.
Relationship to #1613
For the aliased-project-local-helper case specifically, this fix produces the same end behavior as pre-#1613: such helpers get bundled inline. The mechanism is different (we still run the esbuild fallback resolver, but use its result to choose "let esbuild bundle this" rather than "externalize"), so this is not a literal revert:
node_modulesare still detected by the fallback and bundled inline (test atswc-esbuild-plugin.test.ts:161covers this). Pre-fix(builders): resolve path aliases when externalizing non-step imports #1613, these were also bundled but only becauseenhanced-resolvehappened to fail on them; the new path is more deliberate.!esbuildResult.externalguard, which this fix preserves.pluginData?.skipSwcPluginre-entry guard added by fix(builders): resolve path aliases when externalizing non-step imports #1613 is preserved.#1613's stated vitest motivation (externalize aliased imports so
vi.mock()can intercept them) appears to have been illusory — perworkbench/vitest/MOCKING.md,vi.mock()cannot intercept step dependencies regardless of externalization, due to three independent architectural layers (bundle bypasses Vitest's module system, setup runs before mocks, etc.). So we lose nothing user-visible by reverting to inline bundling here.Tests
'rewrites path-aliased imports to relative paths'→ renamed to'bundles path-aliased project-local imports inline', asserts inlined bundling.'bundles transitive aliased imports inside aliased helpers (Mux self-referencing package regression)'reproducing the exact Mux scenario: step → aliased helper → transitive aliased helper.All 133 builder tests pass; typecheck clean.
Trade-off
Aliased helpers now contribute to step bundle size (inlined into each step bundle that imports them). This restores pre-#1613 behavior for that path. Relative-path imports continue to externalize as before, so this only affects projects using path aliases for project-local helpers.