From e6b2819f0f2c261130023f3e896cd04bc164d976 Mon Sep 17 00:00:00 2001 From: Nathan Colosimo <110621881+NathanColosimo@users.noreply.github.com> Date: Tue, 11 Aug 2026 13:14:30 -0700 Subject: [PATCH] fix(next): converge workflow HMR rebuilds --- .changeset/fix-next-hmr-build-race.md | 5 + packages/core/e2e/dev.test.ts | 47 ++- packages/next/src/builder-eager.ts | 350 +++++++----------- packages/next/src/watch-rebuild.test.ts | 366 ++++++++----------- packages/next/src/watch-rebuild.ts | 448 ++++++------------------ 5 files changed, 409 insertions(+), 807 deletions(-) create mode 100644 .changeset/fix-next-hmr-build-race.md diff --git a/.changeset/fix-next-hmr-build-race.md b/.changeset/fix-next-hmr-build-race.md new file mode 100644 index 0000000000..f2e1773854 --- /dev/null +++ b/.changeset/fix-next-hmr-build-race.md @@ -0,0 +1,5 @@ +--- +'@workflow/next': patch +--- + +Preserve source changes made during Next.js development rebuilds for the next HMR pass. diff --git a/packages/core/e2e/dev.test.ts b/packages/core/e2e/dev.test.ts index f5f3dc610b..c87f9ec02a 100644 --- a/packages/core/e2e/dev.test.ts +++ b/packages/core/e2e/dev.test.ts @@ -154,6 +154,7 @@ export function createDevTests(config?: DevTestConfig) { skip: 'workflow dev hmr: skip', hot: 'workflow dev hmr: hot rebuild', full: 'workflow dev hmr: full rediscovery', + idle: 'workflow dev hmr: idle', }; const fetchWithTimeout = (pathname: string) => { @@ -200,6 +201,13 @@ export function createDevTests(config?: DevTestConfig) { const countLogMessage = (log: string, message: string) => log.split(message).length - 1; type ExpectedHmrLogCount = number | { min?: number; max?: number }; + type ExpectedHmrLogCounts = + | 'any' + | { + skip?: ExpectedHmrLogCount; + hot?: ExpectedHmrLogCount; + full?: ExpectedHmrLogCount; + }; const expectLogCount = ( actual: number, expected: ExpectedHmrLogCount | undefined @@ -221,11 +229,7 @@ export function createDevTests(config?: DevTestConfig) { }; const expectHmrLogCounts = async ( cursor: number | undefined, - expected: { - skip?: ExpectedHmrLogCount; - hot?: ExpectedHmrLogCount; - full?: ExpectedHmrLogCount; - } + expected: ExpectedHmrLogCounts ) => { if (cursor === undefined) { return; @@ -236,6 +240,20 @@ export function createDevTests(config?: DevTestConfig) { intervalMs: 250, check: async () => { const log = (await readDevServerLog()).slice(cursor); + expect(log).toContain(hmrLogMessages.idle); + if (expected === 'any') { + expect( + [ + hmrLogMessages.skip, + hmrLogMessages.hot, + hmrLogMessages.full, + ].reduce( + (count, message) => count + countLogMessage(log, message), + 0 + ) + ).toBeGreaterThan(0); + return; + } expectLogCount( countLogMessage(log, hmrLogMessages.skip), expected.skip @@ -909,6 +927,7 @@ ${apiFileContent}` } await waitForHmrReady(); + const setupLogCursor = await readDevServerLogCursor(); const writeFuzzSources = async (iteration: number) => { await Promise.all([ @@ -1025,6 +1044,15 @@ ${apiFileContent}` }, }); assert(workflow); + await pollUntil({ + description: 'HMR fuzz fixture rebuilds to finish', + timeoutMs: flowRouteHmrRediscoveryTimeoutMs, + intervalMs: 250, + check: async () => { + const log = (await readDevServerLog()).slice(setupLogCursor); + expect(log).toContain(hmrLogMessages.idle); + }, + }); const runWorkflow = async () => { const run = await start< [], @@ -1062,7 +1090,7 @@ ${apiFileContent}` { file: files.step, kind: 'none', - expectedLogCounts: { skip: 1 }, + expectedLogCounts: 'any', expectedStepValue: (iteration: number) => `step-only-${iteration}`, source: ( iteration: number @@ -1320,6 +1348,9 @@ export async function hmrFuzzAddedWorkflow() { }, { description: 'workflow file added through API import', + expectedLogCounts: { + full: { min: 1, max: 2 }, + }, write: async (iteration: number) => { await fs.writeFile( files.addedWorkflow, @@ -1353,7 +1384,9 @@ ${apiFileContent}` }, { description: 'workflow file removed from API import', - expectedLogCounts: { full: 1, skip: 1 }, + expectedLogCounts: { + full: { min: 1, max: 2 }, + }, write: async () => { await fs.rm(files.addedWorkflow, { force: true }); await fs.writeFile( diff --git a/packages/next/src/builder-eager.ts b/packages/next/src/builder-eager.ts index 029601767c..1e15ad32fb 100644 --- a/packages/next/src/builder-eager.ts +++ b/packages/next/src/builder-eager.ts @@ -19,11 +19,10 @@ import type { NextConfig as ProjectNextConfig } from 'next'; import { createWatchIgnorePredicate } from './watch-ignore.js'; import { classifyRebuild, + createRebuildScheduler, createSourceSnapshot, - type FileChanges, getRelevantFiles, - pinBaselinesAcrossFullRebuild, - replaceSourceSnapshots, + readSourceSnapshots, type SourceSnapshot, } from './watch-rebuild.js'; @@ -152,7 +151,8 @@ export async function getNextBuilderEager( ? pathname : resolve(this.config.workingDir, pathname) ).replace(/\\/g, '/'); - const sourceSnapshots = new Map(); + let sourceSnapshots = new Map(); + let buildInProgress = false; const watchableExtensions = new Set([ '.js', @@ -190,25 +190,15 @@ export async function getNextBuilderEager( return isIgnoredWatchPath(normalizedPath); }; - let rebuildQueue = Promise.resolve(); - - const enqueue = (task: () => Promise) => { - rebuildQueue = rebuildQueue.then(task).catch((error) => { - console.error('Failed to process file change', error); - }); - return rebuildQueue; - }; - const readSourceSnapshot = (file: string) => createSourceSnapshot({ file, detectWorkflowPatterns }); - const refreshSourceSnapshots = () => - replaceSourceSnapshots({ + const snapshotSources = () => + readSourceSnapshots({ discoveredEntries, inputFiles: options.inputFiles, normalizePath, readSnapshot: readSourceSnapshot, - sourceSnapshots, }); const mergeCombinedManifest = ( @@ -225,6 +215,15 @@ export async function getNextBuilderEager( }, }); + const runBuild = async (build: () => Promise) => { + buildInProgress = true; + try { + await build(); + } finally { + buildInProgress = false; + } + }; + const hotRebuild = async (refreshStepRegistrations: boolean) => { if (refreshStepRegistrations) { if (stepsCtx) { @@ -251,54 +250,42 @@ export async function getNextBuilderEager( await writeManifest(mergeCombinedManifest(stepsManifest)); }; - // The pin helper owns the capture-before-build / restore-after- - // refresh ordering (including that the capture reads the CURRENT - // discovered entries and input files, before the rebuild replaces - // them), so an edit landing while the multi-second rebuild runs still - // diffs against what the rebuild consumed instead of being absorbed - // into the refreshed baseline. See `pinBaselinesAcrossFullRebuild` - // for the full reasoning. - const fullRebuild = () => - pinBaselinesAcrossFullRebuild({ - discoveredEntries, - inputFiles: options.inputFiles, - normalizePath, - readSnapshot: readSourceSnapshot, - sourceSnapshots, - rebuild: async () => { - this.clearDiscoveredEntriesCache(); - const newInputFiles = await this.getInputFiles(); - options.inputFiles = newInputFiles; - - await stepsCtx?.dispose(); - await workflowsCtx.interimBundleCtx.dispose(); - - const newCombined = await this.buildCombinedFunction(options); - stepsCtx = newCombined.stepsContext; - discoveredEntries = newCombined.discoveredEntries; - stepsManifest = newCombined.stepsManifest; - workflowsManifest = newCombined.workflowsManifest; - - if (!newCombined?.interimBundleCtx || !newCombined?.bundleFinal) { - throw new Error( - 'Invariant: expected workflows bundle context after rebuild' - ); - } - workflowsCtx = { - interimBundleCtx: newCombined.interimBundleCtx, - bundleFinal: newCombined.bundleFinal, - }; - - await writeManifest(newCombined.manifest); - await refreshSourceSnapshots(); - }, - }); + const fullRebuild = async () => { + this.clearDiscoveredEntriesCache(); + const newInputFiles = await this.getInputFiles(); + options.inputFiles = newInputFiles; + + // Snapshot before building so edits made during the build remain + // dirty and trigger the file event already queued behind this task. + const nextSourceSnapshots = await snapshotSources(); + + await stepsCtx?.dispose(); + await workflowsCtx.interimBundleCtx.dispose(); + + const newCombined = await this.buildCombinedFunction(options); + stepsCtx = newCombined.stepsContext; + discoveredEntries = newCombined.discoveredEntries; + stepsManifest = newCombined.stepsManifest; + workflowsManifest = newCombined.workflowsManifest; + + if (!newCombined?.interimBundleCtx || !newCombined?.bundleFinal) { + throw new Error( + 'Invariant: expected workflows bundle context after rebuild' + ); + } + workflowsCtx = { + interimBundleCtx: newCombined.interimBundleCtx, + bundleFinal: newCombined.bundleFinal, + }; + + await writeManifest(newCombined.manifest); + sourceSnapshots = nextSourceSnapshots; + }; const isWatchableFile = (path: string) => watchableExtensions.has(extname(path)); - const readKnownFiles = async () => { - const files = new Set(); + const readKnownFileAliases = async () => { const aliases = new Map(); const relevantFiles = getRelevantFiles({ discoveredEntries, @@ -315,7 +302,6 @@ export async function getNextBuilderEager( const canonicalPath = relevantFiles.has(realFilePath) ? realFilePath : filePath; - files.add(canonicalPath); aliases.set(filePath, canonicalPath); aliases.set(realFilePath, canonicalPath); return canonicalPath; @@ -363,205 +349,105 @@ export async function getNextBuilderEager( }; await visit(this.config.workingDir); - return { files, aliases, addKnownFile }; - }; - - const mergeFileChanges = ( - left: FileChanges, - right: FileChanges - ): FileChanges => ({ - addedFiles: unique([...left.addedFiles, ...right.addedFiles]), - modifiedFiles: unique([ - ...left.modifiedFiles, - ...right.modifiedFiles, - ]), - removedFiles: unique([...left.removedFiles, ...right.removedFiles]), - }); - - const unique = (paths: string[]) => [...new Set(paths)]; - - const classifyFileChanges = ({ - changedFiles, - knownFiles, - removedFiles, - }: { - changedFiles: string[]; - knownFiles: Set; - removedFiles: string[]; - }): FileChanges => { - const addedFiles: string[] = []; - const modifiedFiles: string[] = []; - - for (const file of unique(changedFiles)) { - if (knownFiles.has(file)) { - modifiedFiles.push(file); - } else { - addedFiles.push(file); - knownFiles.add(file); - } - } - - for (const file of removedFiles) { - knownFiles.delete(file); - } - - return { - addedFiles, - modifiedFiles, - removedFiles: unique(removedFiles), - }; + return { aliases, addKnownFile }; }; - const hasFileChanges = ({ - addedFiles, - modifiedFiles, - removedFiles, - }: FileChanges) => - addedFiles.length > 0 || - modifiedFiles.length > 0 || - removedFiles.length > 0; const logDevHmr = (...args: unknown[]) => { if (process.env.WORKFLOW_DEV_HMR_LOGS === '1') { console.log(...args); } }; - // Known gap: the initial build has the same two-read shape (the - // combined build above consumed sources, and this refresh re-reads - // them), but no pinning — and the watcher below attaches with - // `ignoreInitial: true`, so an edit landing inside the startup window - // is absorbed with no straggler event to recover it. Bounded by dev - // server startup rather than recurring per rebuild; knowingly out of - // scope for the mid-rebuild pinning above. - await refreshSourceSnapshots(); - let { - files: knownFiles, - aliases: knownFileAliases, - addKnownFile: rememberKnownFile, - } = await readKnownFiles(); + sourceSnapshots = await snapshotSources(); + let { aliases: knownFileAliases, addKnownFile: rememberKnownFile } = + await readKnownFileAliases(); const refreshKnownFiles = async () => { - const nextKnown = await readKnownFiles(); - knownFiles = nextKnown.files; + const nextKnown = await readKnownFileAliases(); knownFileAliases = nextKnown.aliases; rememberKnownFile = nextKnown.addKnownFile; }; - const processFileChanges = async (fileChanges: FileChanges) => { - if (!hasFileChanges(fileChanges)) { - return; - } + const runFullRebuild = async () => { + logDevHmr('workflow dev hmr: full rediscovery'); + await runBuild(fullRebuild); + await refreshKnownFiles(); + }; + const processFileChanges = async (files: string[]) => { const decision = await classifyRebuild({ + files, discoveredEntries, - fileChanges, inputFiles: options.inputFiles, normalizePath, parentHasChild, readSnapshot: readSourceSnapshot, sourceSnapshots, }); - if (decision.kind === 'none') { - logDevHmr('workflow dev hmr: skip'); - for (const [file, snapshot] of decision.snapshots || []) { - sourceSnapshots.set(file, snapshot); - } - return; - } - if (decision.kind === 'full') { - logDevHmr('workflow dev hmr: full rediscovery'); - await fullRebuild(); - await refreshKnownFiles(); - return; - } - - logDevHmr( - `workflow dev hmr: hot rebuild${decision.refreshStepRegistrations ? ' with step registration refresh' : ''}` - ); - await hotRebuild(decision.refreshStepRegistrations); - for (const [file, snapshot] of decision.snapshots) { - sourceSnapshots.set(file, snapshot); + switch (decision.kind) { + case 'skip': + logDevHmr('workflow dev hmr: skip'); + break; + case 'hot': + logDevHmr( + `workflow dev hmr: hot rebuild${decision.refreshStepRegistrations ? ' with step registration refresh' : ''}` + ); + await runBuild(() => + hotRebuild(decision.refreshStepRegistrations) + ); + break; + case 'full': + await runFullRebuild(); + return; + default: + decision satisfies never; + throw new Error('Unknown rebuild decision'); } + sourceSnapshots = new Map([ + ...sourceSnapshots, + ...decision.snapshots, + ]); }; - let pendingFileChanges: FileChanges = { - addedFiles: [], - modifiedFiles: [], - removedFiles: [], + const scheduleRebuild = createRebuildScheduler( + async (request) => { + try { + switch (request.kind) { + case 'files': + await processFileChanges(request.files); + return; + case 'full': + await runFullRebuild(); + return; + default: + request satisfies never; + throw new Error('Unknown scheduled rebuild'); + } + } catch (error) { + console.error('Failed to process file change', error); + } + }, + () => logDevHmr('workflow dev hmr: idle') + ); + const scheduleFileChange = (file: string) => { + scheduleRebuild({ kind: 'files', files: [file] }); }; - let flushTimer: ReturnType | undefined; - - const scheduleFileChanges = (fileChanges: FileChanges) => { - pendingFileChanges = mergeFileChanges( - pendingFileChanges, - fileChanges - ); - if (flushTimer) { - return; + const scheduleBuildOverlap = () => { + if (buildInProgress) { + scheduleRebuild({ kind: 'full' }); } - flushTimer = setTimeout(() => { - const fileChanges = pendingFileChanges; - pendingFileChanges = { - addedFiles: [], - modifiedFiles: [], - removedFiles: [], - }; - flushTimer = undefined; - enqueue(() => processFileChanges(fileChanges)); - }, 10); }; - const resolveExistingEventPath = async (pathname: string) => { + const handleFileWritten = async (pathname: string) => { const normalizedPath = normalizePath(pathname); if (!isWatchableFile(normalizedPath)) { return; } - const knownPath = knownFileAliases.get(normalizedPath); - if (knownPath) { - return knownPath; - } - - try { - const realFilePath = normalizePath(await realpath(normalizedPath)); - return knownFileAliases.get(realFilePath) ?? normalizedPath; - } catch { - return normalizedPath; - } - }; - - const handleFileAdded = async (pathname: string) => { - const normalizedPath = normalizePath(pathname); - if (!isWatchableFile(normalizedPath)) { - return; - } - - const existingPath = await resolveExistingEventPath(normalizedPath); - const wasKnown = existingPath ? knownFiles.has(existingPath) : false; - const canonicalPath = await rememberKnownFile(normalizedPath); - knownFiles.add(canonicalPath); - scheduleFileChanges({ - addedFiles: wasKnown ? [] : [canonicalPath], - modifiedFiles: wasKnown ? [canonicalPath] : [], - removedFiles: [], - }); - }; - - const handleFileChanged = async (pathname: string) => { - const canonicalPath = await resolveExistingEventPath(pathname); - if (!canonicalPath) { - return; - } - - const fileChanges = classifyFileChanges({ - changedFiles: [canonicalPath], - knownFiles, - removedFiles: [], - }); - if (!knownFileAliases.has(canonicalPath)) { - await rememberKnownFile(canonicalPath); - } - scheduleFileChanges(fileChanges); + const canonicalPath = + knownFileAliases.get(normalizedPath) ?? + (await rememberKnownFile(normalizedPath)); + scheduleFileChange(canonicalPath); }; const handleFileRemoved = (pathname: string) => { @@ -572,13 +458,8 @@ export async function getNextBuilderEager( const canonicalPath = knownFileAliases.get(normalizedPath) ?? normalizedPath; - const fileChanges = classifyFileChanges({ - changedFiles: [], - knownFiles, - removedFiles: [canonicalPath], - }); knownFileAliases.delete(normalizedPath); - scheduleFileChanges(fileChanges); + scheduleFileChange(canonicalPath); }; const watcher = chokidar.watch(this.config.workingDir, { @@ -595,12 +476,15 @@ export async function getNextBuilderEager( }); watcher.on('add', (pathname) => { - void handleFileAdded(pathname); + scheduleBuildOverlap(); + void handleFileWritten(pathname); }); watcher.on('change', (pathname) => { - void handleFileChanged(pathname); + scheduleBuildOverlap(); + void handleFileWritten(pathname); }); watcher.on('unlink', (pathname) => { + scheduleBuildOverlap(); handleFileRemoved(pathname); }); watcher.on('error', (error) => { diff --git a/packages/next/src/watch-rebuild.test.ts b/packages/next/src/watch-rebuild.test.ts index 85de08e965..7bfe16d801 100644 --- a/packages/next/src/watch-rebuild.test.ts +++ b/packages/next/src/watch-rebuild.test.ts @@ -1,13 +1,74 @@ -import { describe, expect, test } from 'vitest'; +import { afterEach, describe, expect, test, vi } from 'vitest'; import { classifyRebuild, + createRebuildScheduler, createSourceSnapshotFromSource, extractImportSignature, - pinBaselinesAcrossFullRebuild, type SourceSnapshot, stripCommentsFromSource, } from './watch-rebuild.js'; +afterEach(() => { + vi.useRealTimers(); +}); + +describe('watch-rebuild scheduling', () => { + test('merges changes until filesystem writes become quiet', async () => { + vi.useFakeTimers(); + const rebuild = vi.fn(async () => {}); + const schedule = createRebuildScheduler(rebuild, () => {}); + + schedule({ + kind: 'files', + files: ['/app/workflow.ts'], + }); + await vi.advanceTimersByTimeAsync(99); + schedule({ + kind: 'files', + files: ['/app/helper.ts'], + }); + await vi.advanceTimersByTimeAsync(99); + + expect(rebuild).not.toHaveBeenCalled(); + await vi.advanceTimersByTimeAsync(1); + expect(rebuild).toHaveBeenCalledWith({ + kind: 'files', + files: ['/app/workflow.ts', '/app/helper.ts'], + }); + }); + + test('collapses full rebuild requests while a rebuild runs', async () => { + vi.useFakeTimers(); + const firstBuild = Promise.withResolvers(); + const fullBuild = Promise.withResolvers(); + const idle = Promise.withResolvers(); + const requests: string[] = []; + const onIdle = vi.fn(idle.resolve); + const schedule = createRebuildScheduler(async (request) => { + requests.push(request.kind); + if (requests.length === 1) { + await firstBuild.promise; + } else { + fullBuild.resolve(); + } + }, onIdle); + + schedule({ kind: 'full' }); + await vi.advanceTimersByTimeAsync(100); + + schedule({ kind: 'full' }); + schedule({ kind: 'full' }); + await vi.advanceTimersByTimeAsync(100); + expect(onIdle).not.toHaveBeenCalled(); + firstBuild.resolve(); + await fullBuild.promise; + await idle.promise; + + expect(requests).toEqual(['full', 'full']); + expect(onIdle).toHaveBeenCalledOnce(); + }); +}); + const detectWorkflowPatterns = (source: string) => ({ hasDirective: source.includes("'use workflow'") || @@ -101,11 +162,7 @@ export const allWorkflows = { discoveredSerdeFiles: new Set(), discoveredFiles: new Set([pageFile, registryFile, workflowFile]), }, - fileChanges: { - addedFiles: [], - modifiedFiles: [registryFile], - removedFiles: [], - }, + files: [registryFile], inputFiles: [pageFile], parentHasChild: () => false, readSnapshot: async (file) => @@ -135,11 +192,7 @@ export const allWorkflows = {} as const; discoveredSerdeFiles: new Set(), discoveredFiles: new Set([registryFile]), }, - fileChanges: { - addedFiles: [stepFile], - modifiedFiles: [registryFile], - removedFiles: [], - }, + files: [stepFile, registryFile], inputFiles: [registryFile], parentHasChild: () => false, readSnapshot: async (file) => @@ -152,237 +205,112 @@ export const allWorkflows = {} as const; ).resolves.toEqual({ kind: 'full' }); }); - test('an edit landing during a full rebuild is not absorbed into the baseline', async () => { - // Reproduces the flow-route HMR race: a step definition is added to an - // already-discovered step file while a full rebuild is in flight. The - // post-rebuild baseline refresh reads the file from disk (post-edit), so - // without reconciliation the queued watcher event diffs the edit against - // itself and classifies as a no-op — the added step never reaches the - // manifest. - const stepFile = '/app/workflows/hmr-fuzz-step.ts'; - const pageFile = '/app/app/page.tsx'; - const preBuildSource = `export async function hmrFuzzStep() { - 'use step'; - return 'step-value'; -} -`; - const postEditSource = `export async function hmrFuzzStep() { - 'use step'; - return 'step-value'; -} - -export async function hmrFuzzAddedStep() { - 'use step'; - return 'added-step'; + test('fully rebuilds byte-identical workflow notifications', async () => { + const workflowFile = '/app/workflows/example.ts'; + const source = `export async function example() { + 'use workflow'; } `; - const discoveredEntries = { - discoveredSteps: new Set([stepFile]), - discoveredWorkflows: new Set(), - discoveredSerdeFiles: new Set(), - discoveredFiles: new Set([pageFile, stepFile]), - }; - const sources = new Map([ - [stepFile, preBuildSource], - [pageFile, ''], - ]); - const readSnapshot = async (file: string) => - createSourceSnapshotFromSource( - sources.get(file) ?? '', - detectWorkflowPatterns - ); - - const sourceSnapshots = new Map(); - - // Full rebuild: the helper captures what the build reads before invoking - // the rebuild; the edit lands mid-rebuild and the post-rebuild refresh - // reads it from disk. - await pinBaselinesAcrossFullRebuild({ - discoveredEntries, - inputFiles: [pageFile], - readSnapshot, - sourceSnapshots, - rebuild: async () => { - sources.set(stepFile, postEditSource); - sourceSnapshots.set(stepFile, await readSnapshot(stepFile)); - sourceSnapshots.set(pageFile, await readSnapshot(pageFile)); - }, - }); + const snapshot = createSourceSnapshotFromSource( + source, + detectWorkflowPatterns + ); - // The queued watcher event for the edit must still trigger a rebuild. await expect( classifyRebuild({ - discoveredEntries, - fileChanges: { - addedFiles: [], - modifiedFiles: [stepFile], - removedFiles: [], + discoveredEntries: { + discoveredSteps: new Set(), + discoveredWorkflows: new Set([workflowFile]), + discoveredSerdeFiles: new Set(), + discoveredFiles: new Set([workflowFile]), }, - inputFiles: [pageFile], + files: [workflowFile], + inputFiles: [workflowFile], parentHasChild: () => false, - readSnapshot, - sourceSnapshots, + readSnapshot: async () => snapshot, + sourceSnapshots: new Map([[workflowFile, snapshot]]), }) ).resolves.toEqual({ kind: 'full' }); }); - test('a duplicate watcher event landing during a full rebuild stays a no-op', async () => { - // Watchers routinely emit several events for one edit, and the edit that - // triggered a full rebuild is itself a source of such stragglers landing - // mid-rebuild. The straggler carries the same content the rebuild - // consumed, so it must diff equal against the pinned pre-build baseline - // and classify as 'none' — evicting the baseline instead cascades into - // back-to-back full rebuilds. - const stepFile = '/app/workflows/hmr-fuzz-step.ts'; - const pageFile = '/app/app/page.tsx'; - const source = `export async function hmrFuzzStep() { - 'use step'; - return 'step-value'; -} -`; - const discoveredEntries = { - discoveredSteps: new Set([stepFile]), - discoveredWorkflows: new Set(), - discoveredSerdeFiles: new Set(), - discoveredFiles: new Set([pageFile, stepFile]), - }; - const sources = new Map([ - [stepFile, source], - [pageFile, ''], - ]); - const readSnapshot = async (file: string) => - createSourceSnapshotFromSource( - sources.get(file) ?? '', - detectWorkflowPatterns - ); - - // Full rebuild (triggered by the edit that wrote `source`): the helper's - // capture reads that same content before the rebuild, and the refresh - // reads it again after. - const sourceSnapshots = new Map(); - await pinBaselinesAcrossFullRebuild({ - discoveredEntries, - inputFiles: [pageFile], - readSnapshot, - sourceSnapshots, - rebuild: async () => { - sourceSnapshots.set(stepFile, await readSnapshot(stepFile)); - sourceSnapshots.set(pageFile, await readSnapshot(pageFile)); - }, - }); + test('rebuilds relevant files without snapshots', async () => { + const helperFile = '/app/workflows/helper.ts'; - // The baseline survives (pinned, not evicted), so the queued duplicate - // classifies as a no-op instead of another full rebuild. - expect(sourceSnapshots.has(stepFile)).toBe(true); - const decision = await classifyRebuild({ - discoveredEntries, - fileChanges: { - addedFiles: [], - modifiedFiles: [stepFile], - removedFiles: [], - }, - inputFiles: [pageFile], - parentHasChild: () => false, - readSnapshot, - sourceSnapshots, - }); - expect(decision.kind).toBe('none'); + await expect( + classifyRebuild({ + discoveredEntries: { + discoveredSteps: new Set(), + discoveredWorkflows: new Set(), + discoveredSerdeFiles: new Set(), + discoveredFiles: new Set([helperFile]), + }, + files: [helperFile], + inputFiles: [], + parentHasChild: () => false, + readSnapshot: async () => + createSourceSnapshotFromSource( + "export const value = 'helper';\n", + detectWorkflowPatterns + ), + sourceSnapshots: new Map(), + }) + ).resolves.toEqual({ kind: 'full' }); }); - test('a file created mid-rebuild that the build missed still forces a follow-up rebuild', async () => { - // A file the rebuild never discovered has no baseline after the - // post-rebuild refresh either, so its queued add event classifies - // conservatively — no eviction machinery required. - const stepFile = '/app/workflows/newly-created-step.ts'; - const pageFile = '/app/app/page.tsx'; - const source = `export async function newStep() { - 'use step'; - return 'new-step'; -} -`; - const sources = new Map([ - [stepFile, source], - [pageFile, ''], - ]); - const readSnapshot = async (file: string) => - createSourceSnapshotFromSource( - sources.get(file) ?? '', - detectWorkflowPatterns - ); - - // The build that just finished never saw stepFile: it is in neither the - // discovered entries nor the refreshed baseline. - const sourceSnapshots = new Map(); - await pinBaselinesAcrossFullRebuild({ - discoveredEntries: { - discoveredSteps: new Set(), - discoveredWorkflows: new Set(), - discoveredSerdeFiles: new Set(), - discoveredFiles: new Set([pageFile]), - }, - inputFiles: [pageFile], - readSnapshot, - sourceSnapshots, - rebuild: async () => { - sourceSnapshots.set(pageFile, await readSnapshot(pageFile)); - }, - }); + test('rebuilds new files that can introduce graph entries', async () => { + const routeFile = '/app/app/new/route.ts'; await expect( classifyRebuild({ discoveredEntries: { - discoveredSteps: new Set(), - discoveredWorkflows: new Set(), - discoveredSerdeFiles: new Set(), - discoveredFiles: new Set([pageFile]), - }, - fileChanges: { - addedFiles: [stepFile], - modifiedFiles: [], - removedFiles: [], + discoveredSteps: new Set(), + discoveredWorkflows: new Set(), + discoveredSerdeFiles: new Set(), + discoveredFiles: new Set(), }, - inputFiles: [pageFile], + files: [routeFile], + inputFiles: [], parentHasChild: () => false, - readSnapshot, - sourceSnapshots, + readSnapshot: async () => + createSourceSnapshotFromSource( + "import './workflow';\n", + detectWorkflowPatterns + ), + sourceSnapshots: new Map(), }) ).resolves.toEqual({ kind: 'full' }); }); - test('ignores stale add events for already snapshotted files', async () => { - const stepFile = '/app/workflows/hmr-fuzz-step.ts'; - const pageFile = '/app/app/page.tsx'; - const stepSource = `export async function hmrFuzzStep() { - 'use step'; - return 'step-value'; -} -`; - const sourceSnapshots = new Map([ - [ - stepFile, - createSourceSnapshotFromSource(stepSource, detectWorkflowPatterns), - ], - ]); - - const decision = await classifyRebuild({ - discoveredEntries: { - discoveredSteps: new Set([stepFile]), - discoveredWorkflows: new Set(), - discoveredSerdeFiles: new Set(), - discoveredFiles: new Set([pageFile, stepFile]), - }, - fileChanges: { - addedFiles: [stepFile], - modifiedFiles: [], - removedFiles: [], - }, - inputFiles: [pageFile], - parentHasChild: () => false, - readSnapshot: async () => - createSourceSnapshotFromSource(stepSource, detectWorkflowPatterns), - sourceSnapshots, + test('hot rebuilds body changes used by workflows', async () => { + const helperFile = '/app/workflows/helper.ts'; + const workflowFile = '/app/workflows/workflow.ts'; + const previousHelperSnapshot = createSourceSnapshotFromSource( + "export const value = 'before';\n", + detectWorkflowPatterns + ); + const nextHelperSnapshot = createSourceSnapshotFromSource( + "export const value = 'after';\n", + detectWorkflowPatterns + ); + await expect( + classifyRebuild({ + discoveredEntries: { + discoveredSteps: new Set(), + discoveredWorkflows: new Set([workflowFile]), + discoveredSerdeFiles: new Set(), + discoveredFiles: new Set([helperFile, workflowFile]), + }, + files: [helperFile], + inputFiles: [workflowFile], + parentHasChild: (parent, child) => + parent === workflowFile && child === helperFile, + readSnapshot: async () => nextHelperSnapshot, + sourceSnapshots: new Map([[helperFile, previousHelperSnapshot]]), + }) + ).resolves.toEqual({ + kind: 'hot', + refreshStepRegistrations: false, + snapshots: new Map([[helperFile, nextHelperSnapshot]]), }); - - expect(decision.kind).toBe('none'); }); }); diff --git a/packages/next/src/watch-rebuild.ts b/packages/next/src/watch-rebuild.ts index 557a8f7aac..d9eb6bde8d 100644 --- a/packages/next/src/watch-rebuild.ts +++ b/packages/next/src/watch-rebuild.ts @@ -1,3 +1,4 @@ +import { createHash } from 'node:crypto'; import { readFile } from 'node:fs/promises'; export interface DiscoveredEntriesLike { @@ -7,13 +8,12 @@ export interface DiscoveredEntriesLike { discoveredFiles?: Set; } -export interface FileChanges { - addedFiles: string[]; - modifiedFiles: string[]; - removedFiles: string[]; -} +export type ScheduledRebuild = + | { kind: 'files'; files: string[] } + | { kind: 'full' }; export interface SourceSnapshot { + sourceHash: string; importSignature: string; definitionSignature: string; hasDirective: boolean; @@ -21,7 +21,7 @@ export interface SourceSnapshot { } export type RebuildDecision = - | { kind: 'none'; snapshots?: Map } + | { kind: 'skip'; snapshots: Map } | { kind: 'hot'; refreshStepRegistrations: boolean; @@ -216,6 +216,7 @@ export const createSourceSnapshotFromSource = ( const patterns = detectWorkflowPatterns(sourceWithoutComments); return { + sourceHash: createHash('sha256').update(source).digest('base64url'), importSignature: extractImportSignature(sourceWithoutComments), definitionSignature: extractDefinitionSignature(sourceWithoutComments), hasDirective: patterns.hasDirective, @@ -254,55 +255,17 @@ export const getRelevantFiles = ({ ].map(normalizePath) ); -export const replaceSourceSnapshots = async ({ +export const readSourceSnapshots = async ({ discoveredEntries, inputFiles, normalizePath = defaultNormalizePath, readSnapshot, - sourceSnapshots, }: { discoveredEntries: DiscoveredEntriesLike; inputFiles: string[]; normalizePath?: (path: string) => string; readSnapshot: (file: string) => Promise; - sourceSnapshots: Map; }) => { - sourceSnapshots.clear(); - await Promise.all( - [ - ...getRelevantFiles({ - discoveredEntries, - inputFiles, - normalizePath, - }), - ].map(async (file) => { - try { - sourceSnapshots.set(file, await readSnapshot(file)); - } catch { - // Unreadable (e.g. just deleted) files simply stay absent from the - // freshly cleared map. - } - }) - ); -}; - -/** - * Read snapshots for every currently relevant file without mutating the - * shared baseline map. Used by `pinBaselinesAcrossFullRebuild` to capture - * content at rebuild start, so the baseline can later be pinned to what the - * rebuild actually consumed. - */ -const captureSourceSnapshots = async ({ - discoveredEntries, - inputFiles, - normalizePath = defaultNormalizePath, - readSnapshot, -}: { - discoveredEntries: DiscoveredEntriesLike; - inputFiles: string[]; - normalizePath?: (path: string) => string; - readSnapshot: (file: string) => Promise; -}): Promise> => { const snapshots = new Map(); await Promise.all( [ @@ -320,77 +283,7 @@ const captureSourceSnapshots = async ({ return snapshots; }; -/** - * Run a full rebuild while keeping the classifier baseline anchored to the - * content the rebuild consumed. - * - * A full rebuild reads sources twice: once when the bundler consumes them and - * once when the baseline is refreshed from disk afterwards (the `rebuild` - * callback owns both, in that order). An edit that lands between those reads - * would be absorbed into the baseline without ever being built, and its - * queued watcher event would then classify as a no-op — silently dropping - * the change until the next unrelated rebuild. - * - * To prevent that, the relevant files are re-read from disk immediately - * before the rebuild starts, and files present both before and after get - * that captured value restored. A mid-(multi-second-)rebuild edit then still - * diffs against what the rebuild consumed, while a duplicate watcher event - * for content the rebuild already consumed — watchers routinely emit several - * events per edit, the triggering edit included — diffs equal and stays a - * no-op instead of cascading into back-to-back full rebuilds. - * - * The capture costs one serial read of the relevant set (~150-250ms at ~250 - * files) per full rediscovery. A zero-read formulation — cloning the live - * baseline map and pinning the triggering batch to the snapshots - * `classifyRebuild` read — was tried and reverted: writes landing in the - * capture window (test-teardown restores, multi-flush setup bursts) are - * content the imminent build consumes anyway, and the clone un-absorbs them - * into follow-up full rebuilds; with real-world multi-second rebuilds that - * bursts into rebuild chains. The disk capture intentionally coalesces such - * writes into the in-flight rebuild. - * - * Files the rebuild discovered for the first time have no captured content - * and keep their post-build baseline. That is already sound for the two - * cases that matter: a new file the build missed has no baseline at all, so - * its queued add event forces the follow-up rebuild, and a new file the - * build did consume gets a baseline matching what it consumed. What stays - * narrowed rather than closed is a file created and then edited again within - * one rebuild window — eviction-style conservatism was tried against that - * and rejected too: it turned every added file's routine duplicate watcher - * events into redundant full rebuilds. - */ -export const pinBaselinesAcrossFullRebuild = async ({ - discoveredEntries, - inputFiles, - normalizePath = defaultNormalizePath, - readSnapshot, - rebuild, - sourceSnapshots, -}: { - discoveredEntries: DiscoveredEntriesLike; - inputFiles: string[]; - normalizePath?: (path: string) => string; - readSnapshot: (file: string) => Promise; - rebuild: () => Promise; - sourceSnapshots: Map; -}): Promise => { - const preBuildSnapshots = await captureSourceSnapshots({ - discoveredEntries, - inputFiles, - normalizePath, - readSnapshot, - }); - - await rebuild(); - - for (const [file, snapshot] of preBuildSnapshots) { - if (sourceSnapshots.has(file)) { - sourceSnapshots.set(file, snapshot); - } - } -}; - -const didSourceSnapshotChange = ( +const didSourceStructureChange = ( previousSnapshot: SourceSnapshot, nextSnapshot: SourceSnapshot ) => @@ -399,184 +292,58 @@ const didSourceSnapshotChange = ( previousSnapshot.hasDirective !== nextSnapshot.hasDirective || previousSnapshot.hasSerde !== nextSnapshot.hasSerde; -const unique = (paths: string[]) => [...new Set(paths)]; - -const snapshotChangedFile = async ({ - file, - nextSnapshots, - readSnapshot, - sourceSnapshots, -}: { - file: string; - nextSnapshots: Map; - readSnapshot: (file: string) => Promise; - sourceSnapshots: Map; -}) => { - const previousSnapshot = sourceSnapshots.get(file); - if (!previousSnapshot) { - return false; - } - - const nextSnapshot = await readSnapshot(file); - if (didSourceSnapshotChange(previousSnapshot, nextSnapshot)) { - return false; - } - - nextSnapshots.set(file, nextSnapshot); - return true; -}; - -const removedFilesRequireFullRebuild = ({ - discoveredEntries, - inputFiles, - normalizePath, - removedFiles, -}: { - discoveredEntries: DiscoveredEntriesLike; - inputFiles: string[]; - normalizePath: (path: string) => string; - removedFiles: string[]; -}) => { - const relevantFiles = getRelevantFiles({ - discoveredEntries, - inputFiles, - normalizePath, - }); - return removedFiles.some((file) => relevantFiles.has(file)); -}; - -const addedFilesRequireFullRebuild = async ({ - addedFiles, - readSnapshot, -}: { - addedFiles: string[]; - readSnapshot: (file: string) => Promise; -}) => { - for (const file of addedFiles) { - try { - const snapshot = await readSnapshot(file); - if (snapshot.hasDirective || snapshot.hasSerde) { - return true; - } - } catch { - return true; - } - } - return false; -}; +export const createRebuildScheduler = ( + rebuild: (request: ScheduledRebuild) => Promise, + onIdle: () => void +) => { + let pending: ScheduledRebuild | undefined; + let rebuilding = false; + let timer: ReturnType | undefined; -const pruneStaleAddedFiles = async ({ - addedFiles, - readSnapshot, - sourceSnapshots, -}: { - addedFiles: string[]; - readSnapshot: (file: string) => Promise; - sourceSnapshots: Map; -}) => { - const nextAddedFiles: string[] = []; - const snapshots = new Map(); - - for (const file of unique(addedFiles)) { - const previousSnapshot = sourceSnapshots.get(file); - if (!previousSnapshot) { - nextAddedFiles.push(file); - continue; + const flush = async () => { + if (rebuilding || timer || !pending) { + return; } + const request = pending; + pending = undefined; + rebuilding = true; try { - const nextSnapshot = await readSnapshot(file); - if (didSourceSnapshotChange(previousSnapshot, nextSnapshot)) { - nextAddedFiles.push(file); - continue; + await rebuild(request); + } finally { + rebuilding = false; + if (pending && !timer) { + void flush(); + } else if (!pending) { + onIdle(); } - snapshots.set(file, nextSnapshot); - } catch { - nextAddedFiles.push(file); } - } - - return { addedFiles: nextAddedFiles, snapshots }; -}; + }; -const modifiedFilesRequireFullRebuild = async ({ - modifiedFiles, - readSnapshot, - sourceSnapshots, -}: { - modifiedFiles: string[]; - readSnapshot: (file: string) => Promise; - sourceSnapshots: Map; -}) => { - for (const file of unique(modifiedFiles)) { - try { - const nextSnapshot = await readSnapshot(file); - const previousSnapshot = sourceSnapshots.get(file); - if (!previousSnapshot) { - if ( - nextSnapshot.importSignature || - nextSnapshot.definitionSignature || - nextSnapshot.hasDirective || - nextSnapshot.hasSerde - ) { - return true; + return (request: ScheduledRebuild) => { + switch (request.kind) { + case 'files': + if (pending?.kind !== 'full') { + pending = { + kind: 'files', + files: [...new Set([...(pending?.files ?? []), ...request.files])], + }; } - continue; - } - if (didSourceSnapshotChange(previousSnapshot, nextSnapshot)) { - return true; - } - } catch { - return true; + break; + case 'full': + pending = request; + break; + default: + request satisfies never; + throw new Error('Unknown scheduled rebuild'); } - } - return false; -}; - -const getChangedRelevantFiles = ({ - discoveredEntries, - fileChanges, - inputFiles, - normalizePath, -}: { - discoveredEntries: DiscoveredEntriesLike; - fileChanges: FileChanges; - inputFiles: string[]; - normalizePath: (path: string) => string; -}) => { - const relevantFiles = getRelevantFiles({ - discoveredEntries, - inputFiles, - normalizePath, - }); - return unique(fileChanges.modifiedFiles).filter((file) => - relevantFiles.has(file) - ); -}; -const collectHotRebuildSnapshots = async ({ - changedFiles, - readSnapshot, - sourceSnapshots, -}: { - changedFiles: string[]; - readSnapshot: (file: string) => Promise; - sourceSnapshots: Map; -}) => { - const snapshots = new Map(); - for (const file of changedFiles) { - if ( - !(await snapshotChangedFile({ - file, - nextSnapshots: snapshots, - readSnapshot, - sourceSnapshots, - })) - ) { - return; - } - } - return snapshots; + clearTimeout(timer); + timer = setTimeout(() => { + timer = undefined; + void flush(); + }, 100); + }; }; const workflowEntryFilesChanged = ({ @@ -633,16 +400,16 @@ const stepRegistrationsNeedRefresh = ({ }; export const classifyRebuild = async ({ + files, discoveredEntries, - fileChanges, inputFiles, normalizePath = defaultNormalizePath, parentHasChild, readSnapshot, sourceSnapshots, }: { + files: string[]; discoveredEntries: DiscoveredEntriesLike; - fileChanges: FileChanges; inputFiles: string[]; normalizePath?: (path: string) => string; parentHasChild: ( @@ -653,74 +420,59 @@ export const classifyRebuild = async ({ readSnapshot: (file: string) => Promise; sourceSnapshots: Map; }): Promise => { - const prunedAddedFiles = await pruneStaleAddedFiles({ - addedFiles: fileChanges.addedFiles, - readSnapshot, - sourceSnapshots, - }); - const normalizedFileChanges = { - ...fileChanges, - addedFiles: prunedAddedFiles.addedFiles, - }; - - if ( - removedFilesRequireFullRebuild({ - discoveredEntries, - inputFiles, - normalizePath, - removedFiles: normalizedFileChanges.removedFiles, - }) || - (await addedFilesRequireFullRebuild({ - addedFiles: normalizedFileChanges.addedFiles, - readSnapshot, - })) || - (await modifiedFilesRequireFullRebuild({ - modifiedFiles: normalizedFileChanges.modifiedFiles, - readSnapshot, - sourceSnapshots, - })) - ) { - return { kind: 'full' }; - } - - const changedRelevantFiles = getChangedRelevantFiles({ + const relevantFiles = getRelevantFiles({ discoveredEntries, - fileChanges: normalizedFileChanges, inputFiles, normalizePath, }); - if (changedRelevantFiles.length === 0) { - return prunedAddedFiles.snapshots.size > 0 - ? { kind: 'none', snapshots: prunedAddedFiles.snapshots } - : { kind: 'none' }; - } + const snapshots = new Map(); + for (const file of files) { + let nextSnapshot: SourceSnapshot; + try { + nextSnapshot = await readSnapshot(file); + } catch { + if (relevantFiles.has(file)) { + return { kind: 'full' }; + } + continue; + } - try { - const snapshots = await collectHotRebuildSnapshots({ - changedFiles: changedRelevantFiles, - readSnapshot, - sourceSnapshots, - }); - if (!snapshots) { + const previousSnapshot = sourceSnapshots.get(file); + if (!previousSnapshot) { + if ( + relevantFiles.has(file) || + nextSnapshot.importSignature || + nextSnapshot.hasDirective || + nextSnapshot.hasSerde + ) { + return { kind: 'full' }; + } + continue; + } + if (didSourceStructureChange(previousSnapshot, nextSnapshot)) { return { kind: 'full' }; } - return workflowEntryFilesChanged({ - changedFiles: changedRelevantFiles, - discoveredEntries, - normalizePath, - parentHasChild, - }) - ? { - kind: 'hot', - refreshStepRegistrations: stepRegistrationsNeedRefresh({ - changedFiles: changedRelevantFiles, - discoveredEntries, - normalizePath, - }), - snapshots, - } - : { kind: 'none', snapshots }; - } catch { - return { kind: 'full' }; + if (previousSnapshot.sourceHash === nextSnapshot.sourceHash) { + return { kind: 'full' }; + } + snapshots.set(file, nextSnapshot); } + + const changedFiles = [...snapshots.keys()]; + return workflowEntryFilesChanged({ + changedFiles, + discoveredEntries, + normalizePath, + parentHasChild, + }) + ? { + kind: 'hot', + refreshStepRegistrations: stepRegistrationsNeedRefresh({ + changedFiles, + discoveredEntries, + normalizePath, + }), + snapshots, + } + : { kind: 'skip', snapshots }; };