fix(next): watch before initial build - #3470
NathanColosimo wants to merge 7 commits into
Conversation
🦋 Changeset detectedLatest commit: 43bf5b3 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
|
| Passed | Failed | Skipped | Total | |
|---|---|---|---|---|
| ✅ ▲ Vercel Production | 3662 | 0 | 685 | 4347 |
| ✅ 💻 Local Development | 3998 | 0 | 510 | 4508 |
| ✅ 📦 Local Production | 3998 | 0 | 510 | 4508 |
| ✅ 🐘 Local Postgres | 3998 | 0 | 510 | 4508 |
| ✅ 🪟 Windows | 320 | 0 | 2 | 322 |
| ✅ 🌐 Cross-language Conformance | 68 | 0 | 74 | 142 |
| ✅ vercel-http-transport | 823 | 0 | 143 | 966 |
| ✅ vercel-multi-region | 27 | 0 | 0 | 27 |
| ✅ vercel-ws-transport | 557 | 0 | 87 | 644 |
| Total | 17451 | 0 | 2521 | 19972 |
Details by Category
✅ ▲ Vercel Production
| App | Passed | Failed | Skipped |
|---|---|---|---|
| ✅ astro-node | 133 | 0 | 28 |
| ✅ astro-quickjs | 133 | 0 | 28 |
| ✅ example-node | 133 | 0 | 28 |
| ✅ example-quickjs | 133 | 0 | 28 |
| ✅ express-node | 133 | 0 | 28 |
| ✅ express-quickjs | 133 | 0 | 28 |
| ✅ fastify-node | 133 | 0 | 28 |
| ✅ fastify-quickjs | 133 | 0 | 28 |
| ✅ hono-node | 133 | 0 | 28 |
| ✅ hono-quickjs | 133 | 0 | 28 |
| ✅ nest-node | 133 | 0 | 28 |
| ✅ nest-quickjs | 133 | 0 | 28 |
| ✅ nextjs-turbopack-node | 158 | 0 | 3 |
| ✅ nextjs-turbopack-quickjs | 158 | 0 | 3 |
| ✅ nextjs-webpack-node | 158 | 0 | 3 |
| ✅ nextjs-webpack-quickjs | 158 | 0 | 3 |
| ✅ nitro-node | 133 | 0 | 28 |
| ✅ nitro-quickjs | 133 | 0 | 28 |
| ✅ nuxt-node | 133 | 0 | 28 |
| ✅ nuxt-quickjs | 133 | 0 | 28 |
| ✅ python-node | 66 | 0 | 95 |
| ✅ sveltekit-node | 152 | 0 | 9 |
| ✅ sveltekit-quickjs | 152 | 0 | 9 |
| ✅ tanstack-start-node | 133 | 0 | 28 |
| ✅ tanstack-start-quickjs | 133 | 0 | 28 |
| ✅ vite-node | 133 | 0 | 28 |
| ✅ vite-quickjs | 133 | 0 | 28 |
✅ 💻 Local Development
| App | Passed | Failed | Skipped |
|---|---|---|---|
| ✅ astro-stable-node | 134 | 0 | 27 |
| ✅ astro-stable-quickjs | 134 | 0 | 27 |
| ✅ express-stable-node | 134 | 0 | 27 |
| ✅ express-stable-quickjs | 134 | 0 | 27 |
| ✅ fastify-stable-node | 134 | 0 | 27 |
| ✅ fastify-stable-quickjs | 134 | 0 | 27 |
| ✅ hono-stable-node | 134 | 0 | 27 |
| ✅ hono-stable-quickjs | 134 | 0 | 27 |
| ✅ nest-stable-node | 134 | 0 | 27 |
| ✅ nest-stable-quickjs | 134 | 0 | 27 |
| ✅ nextjs-turbopack-canary-node | 160 | 0 | 1 |
| ✅ nextjs-turbopack-canary-quickjs | 160 | 0 | 1 |
| ✅ nextjs-turbopack-stable-node | 160 | 0 | 1 |
| ✅ nextjs-turbopack-stable-quickjs | 160 | 0 | 1 |
| ✅ nextjs-webpack-canary-node | 160 | 0 | 1 |
| ✅ nextjs-webpack-canary-quickjs | 160 | 0 | 1 |
| ✅ nextjs-webpack-stable-node | 160 | 0 | 1 |
| ✅ nextjs-webpack-stable-quickjs | 160 | 0 | 1 |
| ✅ nitro-stable-node | 134 | 0 | 27 |
| ✅ nitro-stable-quickjs | 134 | 0 | 27 |
| ✅ nuxt-stable-node | 134 | 0 | 27 |
| ✅ nuxt-stable-quickjs | 134 | 0 | 27 |
| ✅ sveltekit-stable-node | 153 | 0 | 8 |
| ✅ sveltekit-stable-quickjs | 153 | 0 | 8 |
| ✅ tanstack-start-node | 134 | 0 | 27 |
| ✅ tanstack-start-quickjs | 134 | 0 | 27 |
| ✅ vite-stable-node | 134 | 0 | 27 |
| ✅ vite-stable-quickjs | 134 | 0 | 27 |
✅ 📦 Local Production
| App | Passed | Failed | Skipped |
|---|---|---|---|
| ✅ astro-stable-node | 134 | 0 | 27 |
| ✅ astro-stable-quickjs | 134 | 0 | 27 |
| ✅ express-stable-node | 134 | 0 | 27 |
| ✅ express-stable-quickjs | 134 | 0 | 27 |
| ✅ fastify-stable-node | 134 | 0 | 27 |
| ✅ fastify-stable-quickjs | 134 | 0 | 27 |
| ✅ hono-stable-node | 134 | 0 | 27 |
| ✅ hono-stable-quickjs | 134 | 0 | 27 |
| ✅ nest-stable-node | 134 | 0 | 27 |
| ✅ nest-stable-quickjs | 134 | 0 | 27 |
| ✅ nextjs-turbopack-canary-node | 160 | 0 | 1 |
| ✅ nextjs-turbopack-canary-quickjs | 160 | 0 | 1 |
| ✅ nextjs-turbopack-stable-node | 160 | 0 | 1 |
| ✅ nextjs-turbopack-stable-quickjs | 160 | 0 | 1 |
| ✅ nextjs-webpack-canary-node | 160 | 0 | 1 |
| ✅ nextjs-webpack-canary-quickjs | 160 | 0 | 1 |
| ✅ nextjs-webpack-stable-node | 160 | 0 | 1 |
| ✅ nextjs-webpack-stable-quickjs | 160 | 0 | 1 |
| ✅ nitro-stable-node | 134 | 0 | 27 |
| ✅ nitro-stable-quickjs | 134 | 0 | 27 |
| ✅ nuxt-stable-node | 134 | 0 | 27 |
| ✅ nuxt-stable-quickjs | 134 | 0 | 27 |
| ✅ sveltekit-stable-node | 153 | 0 | 8 |
| ✅ sveltekit-stable-quickjs | 153 | 0 | 8 |
| ✅ tanstack-start-node | 134 | 0 | 27 |
| ✅ tanstack-start-quickjs | 134 | 0 | 27 |
| ✅ vite-stable-node | 134 | 0 | 27 |
| ✅ vite-stable-quickjs | 134 | 0 | 27 |
✅ 🐘 Local Postgres
| App | Passed | Failed | Skipped |
|---|---|---|---|
| ✅ astro-stable-node | 134 | 0 | 27 |
| ✅ astro-stable-quickjs | 134 | 0 | 27 |
| ✅ express-stable-node | 134 | 0 | 27 |
| ✅ express-stable-quickjs | 134 | 0 | 27 |
| ✅ fastify-stable-node | 134 | 0 | 27 |
| ✅ fastify-stable-quickjs | 134 | 0 | 27 |
| ✅ hono-stable-node | 134 | 0 | 27 |
| ✅ hono-stable-quickjs | 134 | 0 | 27 |
| ✅ nest-stable-node | 134 | 0 | 27 |
| ✅ nest-stable-quickjs | 134 | 0 | 27 |
| ✅ nextjs-turbopack-canary-node | 160 | 0 | 1 |
| ✅ nextjs-turbopack-canary-quickjs | 160 | 0 | 1 |
| ✅ nextjs-turbopack-stable-node | 160 | 0 | 1 |
| ✅ nextjs-turbopack-stable-quickjs | 160 | 0 | 1 |
| ✅ nextjs-webpack-canary-node | 160 | 0 | 1 |
| ✅ nextjs-webpack-canary-quickjs | 160 | 0 | 1 |
| ✅ nextjs-webpack-stable-node | 160 | 0 | 1 |
| ✅ nextjs-webpack-stable-quickjs | 160 | 0 | 1 |
| ✅ nitro-stable-node | 134 | 0 | 27 |
| ✅ nitro-stable-quickjs | 134 | 0 | 27 |
| ✅ nuxt-stable-node | 134 | 0 | 27 |
| ✅ nuxt-stable-quickjs | 134 | 0 | 27 |
| ✅ sveltekit-stable-node | 153 | 0 | 8 |
| ✅ sveltekit-stable-quickjs | 153 | 0 | 8 |
| ✅ tanstack-start-node | 134 | 0 | 27 |
| ✅ tanstack-start-quickjs | 134 | 0 | 27 |
| ✅ vite-stable-node | 134 | 0 | 27 |
| ✅ vite-stable-quickjs | 134 | 0 | 27 |
✅ 🪟 Windows
| App | Passed | Failed | Skipped |
|---|---|---|---|
| ✅ nextjs-turbopack-node | 160 | 0 | 1 |
| ✅ nextjs-turbopack-quickjs | 160 | 0 | 1 |
✅ 🌐 Cross-language Conformance
| App | Passed | Failed | Skipped |
|---|---|---|---|
| ✅ python | 68 | 0 | 74 |
✅ vercel-http-transport
| App | Passed | Failed | Skipped |
|---|---|---|---|
| ✅ example | 133 | 0 | 28 |
| ✅ express | 133 | 0 | 28 |
| ✅ hono | 133 | 0 | 28 |
| ✅ nextjs-turbopack | 158 | 0 | 3 |
| ✅ nitro | 133 | 0 | 28 |
| ✅ vite | 133 | 0 | 28 |
✅ vercel-multi-region
| App | Passed | Failed | Skipped |
|---|---|---|---|
| ✅ nextjs-turbopack | 27 | 0 | 0 |
✅ vercel-ws-transport
| App | Passed | Failed | Skipped |
|---|---|---|---|
| ✅ example | 133 | 0 | 28 |
| ✅ express | 133 | 0 | 28 |
| ✅ nextjs-turbopack | 158 | 0 | 3 |
| ✅ vite | 133 | 0 | 28 |
Sim WorldSimulated world deterministic testing for races. Traces 🟠 world-sim scenario book — 1 fail of 41 total
Full trace: |
2da45c4 to
6c4d8ed
Compare
📊 Workflow Benchmarkscommit Backend:
Streams
📈 STSO distribution vs main (inline / queue-hop histograms)1020 steps (inline) Cumulative STSO time: main 158739ms → this run 169811ms (Δ +11072ms, +7%) 📈 CRTT drill-down vs main (RTT distributions & profiles)RTT over stream progress (avg per tenth of stream, bars scaled min→max): RTT by chunk size (avg per log size bin, ~160B → ~12KB serialized, bars scaled min→max): Delivery jitter over stream progress (avg positive CDV per tenth of stream, bars scaled min→max): ℹ️ Metric definitions & methodologyStreams: first-chunk RTT (the stream-open path, before any buffering/backpressure), CRTT percentiles, and worst delivery stall (CDV max). Cells are medians across iterations; per-run values in the artifacts. No 🔴/🟢 marks until targets attach. The collapsed STSO distribution section above buckets every step gap, split inline (same warm process — pure framework overhead) vs queue-hop (fresh process — dispatch, reinit, replay). The collapsed CRTT drill-down: per-variant RTT histograms (fixed log bins, Best/P75/P90/P99 deltas compare against the most recent benchmark run on Metrics — TTFS: time to first step body (in-deployment start() → first step body) · Fan-out TTFS: fan-out time to first step (in-deployment start() → first of the parallel step bodies to complete) · Fan-out TTLS: fan-out time to last step (in-deployment start() → last of the parallel step bodies to complete, i.e. when the Promise.all resolves) · STSO: step-to-step overhead (gap between consecutive step bodies) · WO: workflow overhead (whole-run time outside step bodies, in-deployment anchored) · CRTT: chunk round-trip time (per-chunk write → read latency, one clock domain: deployment → stream backend → same deployment) · CDV: chunk delay variation / delivery jitter (inter-arrival gap minus inter-write gap per seq-adjacent pair; skew-free; the row is each run's MAX positive value, so one stall moves it) Scenarios — step: one trivial no-op step, no stream; no hooks, so the run stays in turbo mode (in-process fast path) · stream: one streaming step; no hooks, so the run stays in turbo mode (in-process fast path) · hook + stream: registers a hook before one step, which exits turbo mode (dispatch path) · 1020 steps: 1020 trivial sequential steps; STSO is measured between consecutive steps in the given step ranges, and WO is the whole-run overhead outside step bodies · Promise.all(100 steps): 100 trivial no-op steps started together in a single Promise.all; Fan-out TTFS is the first of them to complete and Fan-out TTLS the last, both from the in-deployment clientStart, so their gap is the spread the runtime adds across the fan-out · paced control (100/s, 60B): the control: 300 tiny (~60B) deltas metronome-paced at 100/s — zero workload structure, so it reads the transport floor and flush cadence, and disambiguates transport-wide vs workload-specific when a replay row moves · size sweep (100/s, 160B-12KB): same pacing as the control with deltas padded in rotation across seven log-spaced sizes (~160B–12KB) — rotation decouples size from stream position, so it isolates whether chunk size causes latency · replay gateway-gpt-5.4-nano-2000t (1x): raw provider SSE cadence captured at the AI gateway boundary (gpt-5.4-nano, the most popular gateway model; per-token deltas p50 208B = the modal production chunk size), replayed exactly as measured — the typical customer's workload; its CDV is the typical customer's real delivery jitter · replay eve-gpt-5.6-sol-2000t (1x): a captured eve turn (gpt-5.6-sol, the most-used demanding eve model; ~2000 output tokens = production p50 turn length) replayed exactly as measured — eve's envelope protocol re-ships the cumulative message so sizes ramp 142B→13KB; the demanding outlier tenant's reality · replay eve-gpt-5.6-sol-2000t (2x): the same eve capture at 2x — the headroom/stress row; real fast-tier models emit the same chunk sizes at proportionally higher rate, so time compression is a faithful speed model · first chunk (pooled): every run's seq-0 RTT pooled across all stream scenarios — the first chunk precedes any workload differentiation, so pooling samples one shared stream-open path with exact percentiles Replay cadences (semantic sha256) — eve-gpt-5.6-sol-2000t 🔴 marks a percentile over its target (within target is left unmarked). Targets (p75/p90/p99, ms) — TTFS 200/300/600 All timestamps are deployment-side; runs are triggered in-deployment, so the CI runner and api.vercel.com sit outside every measured window. TTFS = Cold starts stay in the numbers (real bursty-workload latency, inflates P75+); Best is the warm floor. |
6c4d8ed to
8c675e5
Compare
fe23836 to
b1b7609
Compare
About these numbersSizes are gzip; parentheses show the change against
|
VaguelySerious
left a comment
There was a problem hiding this comment.
AI review: blocking issues found
| }, | ||
| } | ||
| ); | ||
| const startupWasStable = await closeWatcherOnError(runFullRebuild()); |
There was a problem hiding this comment.
AI Review: Blocking
The startup rediscovery is unconditional, so every next dev start now pays for two full workflow builds instead of one.
Before this PR the startup path built once and then seeded the baseline with a single sourceSnapshots = await snapshotSources(). That line is gone, and runFullRebuild() is called here regardless of whether the watcher saw anything. fullRebuild() starts with clearDiscoveredEntriesCache(), so nothing from the first build is reused.
Measured on workbench/nextjs-webpack (153 workflows, 147 steps) with WORKFLOW_DEV_HMR_LOGS=1 and a probe around buildCombinedFunction, on a run where the watcher emitted zero events:
WFPROBE buildCombinedFunction start 1789426457223
WFPROBE buildCombinedFunction end 1789426457760 (537ms)
workflow dev hmr: full rediscovery
WFPROBE buildCombinedFunction start 1789426458199
WFPROBE buildCombinedFunction end 1789426458490 (291ms)
workflow dev hmr: ready
workflow dev hmr: idle
The user-visible effect is a duplicated ✓ Compiled workflows in … line and roughly 0.3-0.6s of extra startup on this app; it scales with workflow count.
The second build is only needed if something changed while the first one ran, and you already track that:
const startupWasStable =
watchGeneration === 0
? ((sourceSnapshots = await snapshotSources()), true)
: await closeWatcherOnError(runFullRebuild());(or equivalent). That keeps the race fix and drops the cost when there is no race, which is the common case.
There was a problem hiding this comment.
(AI) Re-verified against the current branch head (43bf5b3, after the merge of the updated base). This still stands, unchanged.
builder-eager.ts:231 runs this.buildCombinedFunction(options) for the initial build. builder-eager.ts:534 then runs runFullRebuild() unconditionally, and fullRebuild() opens with clearDiscoveredEntriesCache() at line 402 before calling buildCombinedFunction again at line 410. Nothing from the first build survives the cache clear, so every next dev start pays two full workflow builds even when the watcher emitted zero events.
A note on what makes this harder to fix than it looks, since the author is no longer available to: the obvious fix (build once, then seed the baseline with sourceSnapshots = await snapshotSources()) is the thing this stack deliberately removed. Per the reply on #3333, sourceSnapshots must contain "only bytes read before the accepted build", because a file can change between esbuild reading it and a post-build snapshot reading it, which would swallow the queued invalidation and leave the bundle stale.
So the correct fix is to route the initial build through the same snapshot-then-build ordering fullRebuild() already implements, rather than building at 231 and re-building at 534. That preserves the coverage invariant this PR exists to establish and removes the duplicate build. It is a real change to the startup path, not a one-liner, and it needs the Local Dev lanes to validate.
| ) { | ||
| return; | ||
| } | ||
| watchGeneration++; |
There was a problem hiding this comment.
AI Review: Blocking
watchGeneration++ runs before anything checks whether the path is watchable, so unrelated files can invalidate a startup that was in fact stable.
The dependency watcher created in replaceDependencyWatcher passes depth: 0 but no ignored option, so it reports every file in a dependency directory, not just the relevant ones. isWatchableFile does filter those out, but only later, inside handleFileChanged / scheduleFullRebuild. By then the generation has already moved.
That matters specifically during the startup window: runFullRebuild() captures generation on entry and compares generation === watchGeneration on exit. Any unwatchable write inside a watched dependency directory during that window flips startupWasStable to false and fires another scheduleRebuild({ kind: 'full' }).
The events are not hypothetical. Instrumenting the listener on workbench/nextjs-webpack captured Next's own generated files arriving through the dependency watcher:
workflow dev hmr: ready
workflow dev hmr: idle
✓ Generated AGENTS.md and CLAUDE.md for AI agents. …
WFPROBE event dependencies add …/workbench/nextjs-webpack/AGENTS.md
WFPROBE event dependencies add …/workbench/nextjs-webpack/CLAUDE.md
A control run appending to an unrelated PROBE_NOTE.md in the app root produced no rediscovery, which confirms the source watcher's ignored predicate is doing its job and the dependency watcher is the hole.
Separately, one uninstrumented run of the same app reached four buildCombinedFunction calls and three full rediscovery markers before settling, with no user edit. I did not instrument that particular run, so I can't attribute those extra rediscoveries to this path with certainty, and I'm not claiming it as proof. The mechanism above is established from the code and from the captured events regardless.
Two cheap fixes, and I'd take both:
- Move the watchability filter ahead of the counter, so unwatchable paths never bump the generation.
- Give the dependency watcher an
ignoredpredicate ((p) => !relevantFiles.has(normalizePath(String(p))), or reuseisIgnoredWatchPath) so it stops registering interest in the whole directory.
This also has a downstream effect on the test PR stacked on top of this one, which asserts an exact zero full-rediscovery count for hot-rebuild cases.
There was a problem hiding this comment.
(AI) Re-verified against 43bf5b3. Still stands.
watchGeneration++ is at builder-eager.ts:178, inside attachWatchEvents' 'all' handler, before any watchability test. The only filtering ahead of it is the addDir/unlinkDir skip for mode === 'source'.
The source watcher is largely protected by its own ignored predicate (stats?.isFile() === true && !isSourceFile(normalizedPath)). The dependency watcher is not: replaceDependencyWatcher at line 326 creates it with { depth: 0, ignoreInitial: true, followSymlinks: true } and no ignored option, so every file written in a watched dependency directory bumps the generation. runFullRebuild captures generation on entry (line 447) and compares at line 455, so any such write during the startup window flips startupWasStable to false and fires a third full rebuild on top of the two above.
Smallest fix is to gate the increment on the same predicate the handlers already use, e.g. skip when mode === 'dependencies' && !isWatchableFile(normalizePath(pathname)), or give the dependency watcher an ignored predicate matching the source watcher's. Either is small and independent of the startup-path work in the other blocker.
| await refreshKnownFiles(); | ||
| return buildWasStable && generation === watchGeneration; | ||
| await replaceDependencyWatcher(); | ||
| const finalSnapshots = await snapshotSources(); |
There was a problem hiding this comment.
AI Review: Note
Every full rebuild now stats the source tree three times: once at the top of fullRebuild (nextSourceSnapshots), once at its return to compare, and a third time here.
The second and third reads are separated only by replaceDependencyWatcher(), and they compute nearly the same predicate: fullRebuild compares nextSourceSnapshots against a fresh read, then this compares sourceSnapshots (just assigned nextSourceSnapshots) against another fresh read. On an app with a few hundred entries that is a redundant stat pass per rebuild, and rebuilds happen on every add/unlink.
If the intent is to also catch writes that land during replaceDependencyWatcher(), say so in a comment, because as written it reads like an accident. Otherwise reuse buildWasStable's snapshot.
| const isWatchableFile = (path: string) => | ||
| isSourceFile(path) || relevantFiles.has(path); | ||
| const normalizedDistDir = normalizePath(this.config.distDir); | ||
| const isIgnoredWatchPath = createWatchIgnorePredicate({ |
There was a problem hiding this comment.
AI Review: Nit
createWatchIgnorePredicate moved out of the if (this.config.watch) block, so production builds now construct it too. It calls loadGitignoreMatchers, which walks up to the project root doing synchronous readFileSync on each .gitignore.
The cost is small (a handful of sync reads) and it is only build-time, so this isn't worth reworking the control flow for. But it is a behavior change outside dev mode that the PR doesn't mention, and it makes a production build's output depend on .gitignore parsing that previously never ran there. sourceWatcherCovers is the only non-watch consumer; if it is itself only reachable under watch, the predicate can stay inside the block.
Summary
Testing
Stack
Stack created with GitHub Stacks CLI • Give Feedback 💬