Uh oh!
There was an error while loading. Please reload this page.
Fix duplicate transforms of Nitro build artifacts - #2925
Conversation
🦋 Changeset detectedLatest commit: aaa603e The changes in this PR will be included in the next version bump. This PR includes changesets to release 16 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
✅ vercel-multi-region
|
📊 Workflow Benchmarkscommit Backend:
Avg deltas compare against the most recent benchmark run on Metrics — TTFS: time to first step body execution · STSO: step-to-step overhead (gap between consecutive step bodies) · WO: workflow overhead (time outside step bodies, client start → last step body exit) · SL: stream latency (first chunk write → visible to the reader) Scenarios — stream: one step that streams chunks back to the client; no hooks, so the run stays in turbo mode · hook + stream: registers a hook before the same streaming step, which exits turbo mode · 1020 steps: 1020 trivial sequential steps; STSO is measured between consecutive steps in the given step ranges 🟢/🔴 mark percentiles within/above target. Targets (p75/p90/p99, ms) — TTFS 200/300/600 · SL 50/60/125 · STSO (1-20) 20/30/60 · STSO (101-120) 30/45/90 · STSO (1001-1020) 40/60/120 TTFS/WO compare client vs deployment clocks and SL compares the step runner’s clock vs the client’s (NTP-synced in CI). WO ends at the last step body exit, the closest observable proxy for the final step-completion request. |
85abc14 to
aaa603eCompare
TooTallNate
left a comment
There was a problem hiding this comment.
Reviewed the fix, the exclusion mechanics, and the tests. Approving.
The widening is principled, not a spot fix. Excluding all of buildDir/ (instead of just buildDir/workflow) is the right boundary: everything under Nitro's build directory is generated output — either already-transformed chunks (the re-registration crash from #2894) or build machinery — and user source never lives there, so there's nothing legitimate left to transform. It also strictly subsumes the old exclusion, so the previous failure mode that exclusion guarded ("undefined class references when Nitro's bundler renames variables") stays covered.
Mechanics verified against @workflow/rollup's matcher:exclude entries are prefix-matched after normalizing backslashes on both sides, so the raw replace(/[\\/]+$/, '') + '/' construction (replacing the old pathe join) is cross-platform safe, and the appended trailing slash prevents sibling-directory false positives (/tmp/.nitro-source staying transformed while /tmp/.nitro/... is skipped) — which the regression test pins explicitly. Nice detail that the test drives both transform surfaces (the lazy Vite plugin and the rollup:before plugin) through the real nitro.setup path with a serde class — the actual crash vector — rather than testing the exclusion list in isolation.
Verified locally: 33/33 nitro tests pass (my initial single failure was a worktree artifact — the pre-existing world-target alias test needs @workflow/world-postgres built; unrelated to this change). CI is fully green, including every nitro/nuxt/tanstack-start/vite lane across dev, prod, and postgres — which also empirically answers the one risk I checked for (dev-mode workflow discovery surviving the wider exclusion). Changeset is a correct @workflow/nitro patch.
One non-blocking defense-in-depth thought: this fixes the duplicate transform at the Nitro layer, but the crash mechanism — a second registration IIFE calling a non-configurable defineProperty(classId), with a wrong chunk-path-derived id to boot — will reproduce identically in any future pipeline that re-feeds transformed output. The SWC-emitted registration could be made resilient (skip or alias when the class already carries a classId, similar to how aliasSerializationClass sidesteps the redefine-throw) so the failure mode degrades to a benign no-op instead of 500ing every SSR request. Worth a follow-up issue if one doesn't exist.
Uh oh!
There was an error while loading. Please reload this page.
Signed-off-by: Nathan Colosimo <110621881+NathanColosimo@users.noreply.github.com>
Backport PR opened against |
This reverts the static world-target injection and restores runtime resolution of the world package from `WORKFLOW_TARGET_WORLD`. Because 141 commits have landed on top of #2752, this is a surgical revert rather than a mechanical one. Removed: the `world-target` modules in core/builders/utils, the bundler aliases and `define`s wired through next/nitro/nuxt/astro/sveltekit/rollup/nest, the `@workflow/core/runtime/world-target` export, and the pg-native/node-compat-banner helpers that only existed to support the statically bundled world. Kept (later work that builds on #2752): - `createWorldFromModule` / `WorldFactoryModule` in core (used by world-testing and the nitro dev handler) - `world-init` / `world-init-stub` / `getWorldLazy` and their registration of `getWorld` on globalThis, plus the #3001 isolated route bundle regression test - #2804 (web), #2806 (cli dynamic community backends), #3112 (world factory alias removal), #2988 (nest vercel builder), #2925/#2908 (nitro), #2799 (sveltekit typescript stub is dropped with the injection it existed for), next basePath support, and the windows / canary test timeout tuning Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Summary
Testing
pnpm vitest run packages/nitro/src/index.test.tspnpm --filter @workflow/nitro buildssr.noExternal: true, and@ai-sdk/gateway@4.0.16Closes#2894