Skip to content

fix(next): watch before initial build #3470

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
NathanColosimo wants to merge 7 commits into from
Open

fix(next): watch before initial build #3470

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
240e393
fix(next): watch before initial build
NathanColosimo Aug 11, 2026
e272571
test(next): bound duplicate HMR events
NathanColosimo Aug 26, 2026
ca2854d
fix(next): await complete watcher coverage
NathanColosimo Aug 26, 2026
8eb6173
perf(next): ignore unrelated directory events
NathanColosimo Aug 26, 2026
388ec67
fix(next): mark startup watcher idle
NathanColosimo Aug 26, 2026
0a60628
perf(next): avoid duplicate dependency watches
NathanColosimo Aug 28, 2026
43bf5b3
Merge commit '6ccd1da79d' into peter/wt-3470
VaguelySerious Sep 14, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/next-watch-before-build.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@workflow/next': patch
---

Start the workflow watcher before the initial development build.
6 changes: 4 additions & 2 deletions packages/core/e2e/dev.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -492,7 +492,9 @@ export function createDevTests(config?: DevTestConfig) {
);

await expectGeneratedArtifactsUnchanged(snapshot);
await expectHmrLogCounts(logCursor, { skip: 1 });
await expectHmrLogCounts(logCursor, {
skip: { kind: 'range', min: 1, max: 2 },
});
}
);

Expand Down Expand Up @@ -530,7 +532,7 @@ export async function hmrPageWorkflow() {
},
});
await expectHmrLogCounts(logCursor, {
full: 1,
full: { kind: 'range', min: 1, max: 2 },
skip: { kind: 'range', min: 0, max: 1 },
});
}
Expand Down
274 changes: 182 additions & 92 deletions packages/next/src/builder-eager.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
import assert from 'node:assert';
import { once } from 'node:events';
import { constants } from 'node:fs';
import { access, mkdir, realpath, rm, stat } from 'node:fs/promises';
import { extname, isAbsolute, join, relative, resolve } from 'node:path';
import { dirname, isAbsolute, join, relative, resolve } from 'node:path';
import type {
NextConfig as BuilderNextConfig,
WorkflowManifest,
Expand Down Expand Up @@ -123,8 +124,101 @@ export async function getNextBuilderEager(
}
await writeFileIfChanged(join(workflowGeneratedDir, '.gitignore'), '*');

const inputFiles = await this.getInputFiles();
const tsconfigPath = await this.findTsConfigPath();
const normalizePath = (pathname: string) =>
(isAbsolute(pathname)
? pathname
: resolve(this.config.workingDir, pathname)
).replace(/\\/g, '/');
const normalizedWorkingDir = normalizePath(this.config.workingDir);
let relevantFiles = new Set<string>();
const isWatchableFile = (path: string) =>
isSourceFile(path) || relevantFiles.has(path);
const normalizedDistDir = normalizePath(this.config.distDir);
const isIgnoredWatchPath = createWatchIgnorePredicate({

@VaguelySerious VaguelySerious Sep 14, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

workingDir: this.config.workingDir,
projectRoot: this.transformProjectRoot,
extraFragments: [workflowGeneratedDir.replace(/\\/g, '/')],
});
const sourceWatcherCovers = (file: string) =>
file.startsWith(`${normalizedWorkingDir}/`) &&
!file.startsWith(`${normalizedDistDir}/`) &&
!isIgnoredWatchPath(file) &&
isSourceFile(file);
const logDevHmr = (...args: unknown[]) => {
if (process.env.WORKFLOW_DEV_HMR_LOGS === '1') {
console.log(...args);
}
};

type WatchEvent = {
kind: 'add' | 'addDir' | 'change' | 'unlink' | 'unlinkDir';
pathname: string;
};
let watchGeneration = 0;
let handleWatchEvent = async (_event: WatchEvent) => {};
const attachWatchEvents = (
currentWatcher: ReturnType<typeof chokidar.watch>,
mode: 'source' | 'dependencies'
) => {
currentWatcher.on('all', (kind, pathname) => {
if (
kind === 'all' ||
kind === 'error' ||
kind === 'raw' ||
kind === 'ready'
) {
throw new Error(`Unknown watch event: ${kind}`);
}
if (
mode === 'source' &&
(kind === 'addDir' || kind === 'unlinkDir')
) {
return;
}
watchGeneration++;

@VaguelySerious VaguelySerious Sep 14, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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:

  1. Move the watchability filter ahead of the counter, so unwatchable paths never bump the generation.
  2. Give the dependency watcher an ignored predicate ((p) => !relevantFiles.has(normalizePath(String(p))), or reuse isIgnoredWatchPath) 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.

@VaguelySerious VaguelySerious Sep 14, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(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.

void handleWatchEvent({ kind, pathname }).catch((error) => {
console.error('Failed to process file change', error);
});
});
};
// Chokidar 4 registers an fs.watch per directory, so prune ignored trees
// before it walks the project.
const watcher = this.config.watch
? chokidar.watch(this.config.workingDir, {
ignoreInitial: true,
followSymlinks: true,
ignored: (pathname, stats) => {
const normalizedPath = normalizePath(String(pathname));
if (
normalizedPath === normalizedDistDir ||
normalizedPath.startsWith(`${normalizedDistDir}/`) ||
isIgnoredWatchPath(normalizedPath)
) {
return true;
}
return stats?.isFile() === true && !isSourceFile(normalizedPath);
},
})
: undefined;
let dependencyWatcher: ReturnType<typeof chokidar.watch> | undefined;
const closeWatcherOnError = async <T>(promise: Promise<T>) => {
try {
return await promise;
} catch (error) {
await Promise.all([watcher?.close(), dependencyWatcher?.close()]);
throw error;
}
};
if (watcher) {
attachWatchEvents(watcher, 'source');
watcher.on('error', (error) => {
console.error('Workflow dev watcher error', error);
});
await closeWatcherOnError(once(watcher, 'ready'));
Comment thread
vercel[bot] marked this conversation as resolved.
Resolved
}

const inputFiles = await closeWatcherOnError(this.getInputFiles());
const tsconfigPath = await closeWatcherOnError(this.findTsConfigPath());

const options = {
inputFiles,
Expand All @@ -133,8 +227,12 @@ export async function getNextBuilderEager(
};

// V2: Build combined route (replaces separate step + flow routes)
const combinedResult = await this.buildCombinedFunction(options);
await this.buildWebhookRoute({ workflowGeneratedDir });
const combinedResult = await closeWatcherOnError(
this.buildCombinedFunction(options)
);
await closeWatcherOnError(
this.buildWebhookRoute({ workflowGeneratedDir })
);

const writeManifest = async (
sourceManifest: WorkflowManifest | undefined
Expand Down Expand Up @@ -175,12 +273,14 @@ export async function getNextBuilderEager(
}
};

await writeManifest(combinedResult?.manifest);
await closeWatcherOnError(writeManifest(combinedResult?.manifest));

await this.writeFunctionsConfig(outputDir);
await closeWatcherOnError(this.writeFunctionsConfig(outputDir));

if (this.config.watch) {
assert(watcher, 'Invariant: expected workflow watcher in watch mode');
if (!combinedResult?.interimBundleCtx || !combinedResult.bundleFinal) {
await watcher.close();
throw new Error(
'Invariant: expected workflow build context in watch mode'
);
Expand All @@ -199,43 +299,53 @@ export async function getNextBuilderEager(
'__step_registrations.js'
);

const normalizePath = (pathname: string) =>
(isAbsolute(pathname)
? pathname
: resolve(this.config.workingDir, pathname)
).replace(/\\/g, '/');
let affectedWorkflowFiles = getAffectedWorkflowFiles({
discoveredEntries,
importGraph: discoveredEntries.importParents,
normalizePath,
});
let sourceSnapshots = new Map<string, SourceSnapshot>();

const normalizedGeneratedDir = workflowGeneratedDir.replace(/\\/g, '/');
const normalizedDistDir = normalizePath(this.config.distDir);

// Prune the dev watch set to keep chokidar from registering an
// fs.watch per directory across the whole project tree (chokidar 4
// dropped fsevents, so on macOS that exhausts the fd limit -> EMFILE
// on large monorepos). This honors `.gitignore` and the
// WORKFLOW_DEV_WATCH_IGNORED_PATHS env var in addition to the
// built-in fragments. The generated workflow dir is passed as an
// extra fragment so it is pruned regardless of `.gitignore`.
const isIgnoredWatchPath = createWatchIgnorePredicate({
workingDir: this.config.workingDir,
projectRoot: this.transformProjectRoot,
extraFragments: [normalizedGeneratedDir],
relevantFiles = getRelevantFiles({
discoveredEntries,
inputFiles: options.inputFiles,
normalizePath,
});

const hasIgnoredPathFragment = (normalizedPath: string) => {
if (
normalizedPath === normalizedDistDir ||
normalizedPath.startsWith(`${normalizedDistDir}/`)
) {
return true;
const replaceDependencyWatcher = async () => {
const dependencyDirectories = new Set(
[...relevantFiles]
.filter((file) => !sourceWatcherCovers(file))
.map(dirname)
);
if (dependencyDirectories.size === 0) {
const previousWatcher = dependencyWatcher;
dependencyWatcher = undefined;
await previousWatcher?.close();
return;
}

const nextWatcher = chokidar.watch([...dependencyDirectories], {
depth: 0,
ignoreInitial: true,
followSymlinks: true,
});
attachWatchEvents(nextWatcher, 'dependencies');
nextWatcher.on('error', (error) => {
console.error('Workflow dev watcher error', error);
});
try {
await once(nextWatcher, 'ready');
} catch (error) {
await nextWatcher.close();
throw error;
}
return isIgnoredWatchPath(normalizedPath);

const previousWatcher = dependencyWatcher;
dependencyWatcher = nextWatcher;
await previousWatcher?.close();
};
await closeWatcherOnError(replaceDependencyWatcher());

let sourceSnapshots = new Map<string, SourceSnapshot>();

const readSourceSnapshot = (file: string) =>
createSourceSnapshot({ file, analyzeWorkflowSource });
Expand Down Expand Up @@ -317,6 +427,11 @@ export async function getNextBuilderEager(
interimBundleCtx: newCombined.interimBundleCtx,
bundleFinal: newCombined.bundleFinal,
};
relevantFiles = getRelevantFiles({
discoveredEntries,
inputFiles: options.inputFiles,
normalizePath,
});

await previousWorkflowsCtx.dispose();

Expand All @@ -328,38 +443,17 @@ export async function getNextBuilderEager(
);
};

let relevantFiles = getRelevantFiles({
discoveredEntries,
inputFiles: options.inputFiles,
normalizePath,
});
const isWatchableFile = (path: string) =>
isSourceFile(path) || relevantFiles.has(path);

const logDevHmr = (...args: unknown[]) => {
if (process.env.WORKFLOW_DEV_HMR_LOGS === '1') {
console.log(...args);
}
};

sourceSnapshots = await snapshotSources();
let watchGeneration = 0;

const refreshKnownFiles = async () => {
relevantFiles = getRelevantFiles({
discoveredEntries,
inputFiles: options.inputFiles,
normalizePath,
});
await watcher.add([...relevantFiles]);
};

const runFullRebuild = async () => {
const generation = watchGeneration;
logDevHmr('workflow dev hmr: full rediscovery');
const buildWasStable = await fullRebuild();
await refreshKnownFiles();
return buildWasStable && generation === watchGeneration;
await replaceDependencyWatcher();
const finalSnapshots = await snapshotSources();

@VaguelySerious VaguelySerious Sep 14, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

return (
buildWasStable &&
sourceSnapshotsMatch(sourceSnapshots, finalSnapshots) &&
generation === watchGeneration
);
};

const processFileChanges = async (files: string[]) => {
Expand Down Expand Up @@ -415,7 +509,6 @@ export async function getNextBuilderEager(
() => logDevHmr('workflow dev hmr: idle')
);
const handleFileChanged = async (pathname: string) => {
watchGeneration++;
const normalizedPath = normalizePath(pathname);
if (!isWatchableFile(normalizedPath)) {
return;
Expand All @@ -431,42 +524,39 @@ export async function getNextBuilderEager(
};

const scheduleFullRebuild = (pathname: string) => {
watchGeneration++;
const normalizedPath = normalizePath(pathname);
if (!isWatchableFile(normalizedPath)) {
return;
}
scheduleRebuild({ kind: 'full' });
};

const watcher = chokidar.watch(
[this.config.workingDir, ...relevantFiles],
{
ignoreInitial: true,
followSymlinks: true,
ignored: (pathname) => {
const normalizedPath = normalizePath(String(pathname));
if (relevantFiles.has(normalizedPath)) {
return false;
}
const extension = extname(normalizedPath);
if (extension && !isSourceFile(normalizedPath)) {
return true;
}
return hasIgnoredPathFragment(normalizedPath);
},
}
);
const startupWasStable = await closeWatcherOnError(runFullRebuild());

@VaguelySerious VaguelySerious Sep 14, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@VaguelySerious VaguelySerious Sep 14, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(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.


watcher.on('add', scheduleFullRebuild);
watcher.on('change', handleFileChanged);
watcher.on('unlink', scheduleFullRebuild);
watcher.on('error', (error) => {
console.error('Workflow dev watcher error', error);
});
watcher.on('ready', () => {
logDevHmr('workflow dev hmr: ready');
});
handleWatchEvent = async (event) => {
switch (event.kind) {
case 'add':
case 'unlink':
scheduleFullRebuild(event.pathname);
return;
case 'addDir':
case 'unlinkDir':
scheduleRebuild({ kind: 'full' });
return;
case 'change':
await handleFileChanged(event.pathname);
return;
default:
event.kind satisfies never;
throw new Error('Unknown watch event');
}
};
logDevHmr('workflow dev hmr: ready');
if (startupWasStable) {
logDevHmr('workflow dev hmr: idle');
} else {
scheduleRebuild({ kind: 'full' });
}
}
}

Expand Down
Loading