[next] Suppress duplicate dev-watcher notifications instead of re-invalidating - #3592
Open
pranaygp wants to merge 2 commits into
Open
[next] Suppress duplicate dev-watcher notifications instead of re-invalidating#3592pranaygp wants to merge 2 commits into
pranaygp wants to merge 2 commits into
Conversation
…alidating Watchers routinely emit several events per write (chokidar 4 has no fsevents; node fs.watch double-fires), and the classifier treated every one as a fresh invalidation: a duplicate notification for an already-consumed write became another hot rebuild or full rediscovery. The dev e2e HMR log-count assertions count those, which is the dominant remaining source of E2E Local Dev lane failures on main (observed as "expected 2/3/5 to be 1" across webpack/turbopack, stable and canary). Source snapshots now record the file mtime alongside the content hash, which identifies a specific write: an event whose content AND mtime both match the baseline is a duplicate and classifies as a no-op (logged as "workflow dev hmr: duplicate", which the tests do not count), while identical content with a newer mtime is a distinct rewrite whose interim states an in-flight build may have consumed, and stays a conservative full rediscovery for build-relevant files. Two supporting changes make the suppression airtight: - Full-rediscovery decisions carry every snapshot the classifier read (the whole batch, not just up to the deciding file), and the full rebuild seeds baselines from them for files it discovers for the first time — a freshly created file's duplicate events now diff equal instead of forcing a second rediscovery (the create-then-edit window #3529 documented as narrowed is closed by the mtime rule: a real mid-build edit diffs as changed, a same-content rewrite as a rewrite). - The unconditional schedule-a-full-on-any-event-during-a-build overlap handling is removed: overlapping events already re-classify after the build against the baseline it consumed, and the mtime rule covers the torn-consumption case the overlap full existed for. This eliminates the guaranteed double rediscovery when a duplicate landed mid-build. Irrelevant files that cannot affect the build are now tracked on first sight so their duplicates suppress too instead of re-logging skips. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
🦋 Changeset detectedLatest commit: e93fe3a 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 |
Contributor
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates the Next.js dev watcher rebuild pipeline in @workflow/next to suppress duplicate filesystem notifications (rather than treating them as fresh invalidations), improving determinism of HMR behavior and stabilizing local-dev E2E lanes.
Changes:
- Extend
SourceSnapshotwith a content hash +mtimeMsto distinguish true duplicates from same-content rewrites, and introduce aduplicaterebuild decision. - Rework rebuild scheduling/processing to batch file events and remove “always schedule a follow-up full rebuild on overlap” behavior.
- Update unit and E2E tests to cover duplicate suppression and to wait for an explicit “idle” marker before asserting log counts.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/next/src/watch-rebuild.ts | Adds hash+mtime snapshotting, duplicate decision, and a new rebuild scheduler + batch-based classifier. |
| packages/next/src/watch-rebuild.test.ts | Adds/updates unit coverage for scheduling and duplicate/no-op semantics. |
| packages/next/src/builder-eager.ts | Integrates new scheduler + classifier behavior and adds idle/duplicate dev HMR markers. |
| packages/core/e2e/dev.test.ts | Adjusts HMR log assertions to wait for idle and relaxes expectations in select scenarios. |
| .changeset/hmr-duplicate-event-suppression.md | Changeset describing duplicate watcher suppression behavior. |
| .changeset/fix-next-hmr-build-race.md | Changeset for the stacked/base fix that preserves edits during rebuilds. |
Suppressed comments (1)
packages/core/e2e/dev.test.ts:250
- In the
'any'log-count mode, the “at least one marker happened” check ignores the newduplicatemarker. If an edit is absorbed by an in-flight rebuild and only emitsduplicate+idle, this poll will never succeed even though the system is converged. Include theduplicatemarker in this list.
[
hmrLogMessages.skip,
hmrLogMessages.hot,
hmrLogMessages.full,
].reduce(
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+355
to
359
| if (pending && !timer) { | ||
| void flush(); | ||
| } else if (!pending) { | ||
| onIdle(); | ||
| } |
Comment on lines
465
to
+468
| const canonicalPath = | ||
| knownFileAliases.get(normalizedPath) ?? normalizedPath; | ||
| const fileChanges = classifyFileChanges({ | ||
| changedFiles: [], | ||
| knownFiles, | ||
| removedFiles: [canonicalPath], | ||
| }); | ||
| knownFileAliases.delete(normalizedPath); | ||
| scheduleFileChanges(fileChanges); | ||
| scheduleFileChange(canonicalPath); |
Comment on lines
153
to
+157
| const hmrLogMessages = { | ||
| skip: 'workflow dev hmr: skip', | ||
| hot: 'workflow dev hmr: hot rebuild', | ||
| full: 'workflow dev hmr: full rediscovery', | ||
| idle: 'workflow dev hmr: idle', |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary & Motivation
The E2E Local Dev lanes (nextjs-webpack/nextjs-turbopack × stable/canary × node/quickjs) are the reason
mainhas been red on most runs for days:dev.test.ts's HMR log-count assertions fail withexpected 2/3/5 to be 1(spurious extra rebuilds) orexpected +0 to be 1(a dropped edit). #3529 fixed the mid-rebuild absorption race and #3333 fixes convergence, but the duplicate watcher event mode survives both: watchers routinely emit several events per write (chokidar 4 has no fsevents;fs.watchdouble-fires), and the classifier treats every notification as a fresh invalidation.Reproduced on #3333's own head in 2 local runs: the
body-only changestest failed withexpected 2 to be 1— the server log showsfull rediscovery → build → full rediscoveryfor a singlefs.writeFile, because the duplicate event either lands mid-build (the overlap handler schedules an unconditional follow-up full) or after it (byte-identical content classifies as a conservative full). The same double-full signature is visible in the failing CI runs (e.g. nextjs-webpack stable node, twoskiplines 1.2s apart for one edit).The fix
SourceSnapshotnow records the file mtime alongside the content hash, which identifies a specific write:workflow dev hmr: duplicate(not counted by the e2e assertions).Two supporting changes make the suppression cover every path:
Irrelevant files are now tracked on first sight so their duplicates suppress too instead of re-logging
skip(the{ skip: 1 }exact assertions).Validation
@workflow/nextunit suite: 44/44 (new coverage: duplicate suppression, same-content rewrite stays full, full-decision snapshot seeding, batch mixing duplicates with real changes, irrelevant-file tracking).dev.test.tsruns green (85–120s each). Baseline on this machine: fix(next): converge workflow HMR rebuilds #3333's head failed 1 of 2 runs (expected 2 to be 1); CI on main fails these lanes in the majority of runs.full rediscovery → duplicate → idlewhere the same edit previously producedfull → full.One local-harness caveat discovered on the way, for anyone reproducing: a
.env.localin the workbench (e.g. fromvercel env pull) makesnext dev --turbopackevaluate the config twice and spawn two builder instances — every HMR log line doubles and all exact-count tests fail. CI never has one; delete it locally.🤖 Generated with Claude Code