fix(next): skip eager discovery in deferred build mode#1646
Conversation
🦋 Changeset detectedLatest commit: ed317bd 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 |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📊 Benchmark Results
workflow with no steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Next.js (Turbopack) | Express workflow with 1 step💻 Local Development
▲ Production (Vercel)
🔍 Observability: Express | Next.js (Turbopack) workflow with 10 sequential steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Express | Next.js (Turbopack) workflow with 25 sequential steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Express | Next.js (Turbopack) workflow with 50 sequential steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Express | Next.js (Turbopack) Promise.all with 10 concurrent steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Next.js (Turbopack) | Express Promise.all with 25 concurrent steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Express | Next.js (Turbopack) Promise.all with 50 concurrent steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Express | Next.js (Turbopack) Promise.race with 10 concurrent steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Next.js (Turbopack) | Express Promise.race with 25 concurrent steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Next.js (Turbopack) | Express Promise.race with 50 concurrent steps💻 Local Development
▲ Production (Vercel)
🔍 Observability: Next.js (Turbopack) | Express workflow with 10 sequential data payload steps (10KB)💻 Local Development
▲ Production (Vercel)
🔍 Observability: Next.js (Turbopack) | Express workflow with 25 sequential data payload steps (10KB)💻 Local Development
▲ Production (Vercel)
🔍 Observability: Express | Next.js (Turbopack) workflow with 50 sequential data payload steps (10KB)💻 Local Development
▲ Production (Vercel)
🔍 Observability: Express | Next.js (Turbopack) workflow with 10 concurrent data payload steps (10KB)💻 Local Development
▲ Production (Vercel)
🔍 Observability: Express | Next.js (Turbopack) workflow with 25 concurrent data payload steps (10KB)💻 Local Development
▲ Production (Vercel)
🔍 Observability: Express | Next.js (Turbopack) workflow with 50 concurrent data payload steps (10KB)💻 Local Development
▲ Production (Vercel)
🔍 Observability: Express | Next.js (Turbopack) Stream Benchmarks (includes TTFB metrics)workflow with stream💻 Local Development
▲ Production (Vercel)
🔍 Observability: Express | Next.js (Turbopack) stream pipeline with 5 transform steps (1MB)💻 Local Development
▲ Production (Vercel)
🔍 Observability: Express | Next.js (Turbopack) 10 parallel streams (1MB each)💻 Local Development
▲ Production (Vercel)
🔍 Observability: Express | Next.js (Turbopack) fan-out fan-in 10 streams (1MB each)💻 Local Development
▲ Production (Vercel)
🔍 Observability: Express | 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. |
🧪 E2E Test Results❌ Some tests failed Summary
❌ Failed Tests🌍 Community Worlds (73 failed)mongodb (7 failed):
redis (7 failed):
turso (59 failed):
Details by Category✅ ▲ Vercel Production
✅ 💻 Local Development
✅ 📦 Local Production
✅ 🐘 Local Postgres
✅ 🪟 Windows
❌ 🌍 Community Worlds
✅ 📋 Other
|
TooTallNate
left a comment
There was a problem hiding this comment.
The core change is sound — removing eager input-graph discovery in deferred mode and relying entirely on loader/socket-driven discovery is the right direction. The route stub wait mechanism is well-designed. However, there are a couple of issues to address.
What I verified
Core removal: loadDiscoveredEntriesFromInputGraph() and the overridden getInputFiles() (which filtered to App/Pages Router entrypoints) are cleanly removed. The bootstrap() method no longer calls discovery — entries arrive via loader → socket → onFileDiscovered.
Route stub wait protocol: When the loader encounters a stub file (containing ROUTE_STUB_FILE_MARKER), it opens a dedicated socket, sends trigger-build, and waits for build-complete. This is correct:
createSocketConnectionhas a 1s connect timeoutwaitForDeferredBuildCompletehas a 120s timeoutensureDeferredRouteStubBuildAndWaitdeduplicates concurrent waits viapendingDeferredRouteStubBuildPromise- The socket server's
buildTriggeredflag + immediatebuild-completefor late connectors prevents missed notifications
Loader deduplication: updateDiscoveredPatternState tracks per-file {hasWorkflow, hasStep, hasSerde} and only notifies the socket server when patterns change. This avoids unnecessary rebuild triggers during HMR when files are re-processed without pattern changes.
Manifest step resolution: collectManifestStepSourceFiles resolves relative step entries against multiple base dirs (projectRoot, workingDir, monorepo root). normalizeDiscoveredFilePath now uses realpathSync to resolve symlinks — important for pnpm monorepos.
Build ordering change: Steps build now happens after workflows build (buildWorkflowsFunction → buildStepsFunction), with the workflow manifest passed as additionalStepSourceManifest. This ensures transitive step files discovered by the workflow build are included in the steps bundle.
Blocking (2)
- Socket credential resolution order flipped — see inline comment
getGeneratedRouteStateuses file size as a heuristic — see inline comment
TooTallNate
left a comment
There was a problem hiding this comment.
Both blocking issues from my previous review are resolved in eb1fa370, plus all non-blocking suggestions were addressed in follow-up commits.
| Original Issue | Status | Fix |
|---|---|---|
Socket credential resolution order flipped (file ?? env) |
Fixed (eb1fa370) |
Restored to env ?? file — env vars checked first (always current), file-based fallback for workers |
getGeneratedRouteState file size heuristic |
Fixed (eb1fa370) |
Replaced with bounded 4KB read() — always checks for ROUTE_STUB_FILE_MARKER but limits I/O. Better than my suggestion of reading the whole file |
process.cwd() may differ from builder's workingDir |
Fixed (59763fd6) |
Checks WORKFLOW_PROJECT_ROOT first, falls back to process.cwd() |
getManifestStepResolveBaseDirs Windows safety |
Fixed (d25f9fd4) |
Simplified to add every ancestor dir, normalizing via addResolveBaseDir helper |
LGTM.
Summary
onBeforeDeferredEntrieshandling)@workflow/nextx-ref: #1159 (comment)
Validation
pnpm vitest run packages/next/src/index.test.tsAPP_NAME=nextjs-turbopack pnpm vitest run packages/core/e2e/local-build.test.tsAPP_NAME=nextjs-webpack pnpm vitest run packages/core/e2e/local-build.test.ts