fix: handle watchpack existence-only timestamp entries#20916
Merged
Conversation
`addFileTimestamps` and `addContextTimestamps` accept watchpack-style
`ExistenceOnlyTimeEntry` (`{}`) values, but the cache types previously
narrowed away that variant via cast. Downstream code then treated `{}`
as a fully-populated timestamp, so snapshot validity checks compared
"no info" against snapshots that do have a `timestamp` / `timestampHash`
and falsely invalidated the snapshot — most visibly causing loaders that
call `addContextDependency` to rerun once after the first change.
`FileTimestamp` and `ContextTimestamp` now include `ExistenceOnlyTimeEntry`,
the lying casts in the `add*` methods are gone, and every cache lookup
uses an `isExistenceOnly` check to fall back to a fresh on-disk read
when the cached entry is `{}` or otherwise lacks data the snapshot
expects (e.g. `{ safeTime }` without a `timestampHash`).
Adds:
- `test/watchCases/context/loader-context-dep-no-rerun/` end-to-end
reproduction.
- `FileSystemInfo` unit tests covering `addFileTimestamps` /
`addContextTimestamps` with `{}` and `{ safeTime }` entries, both for
snapshot validity and for `getFileTimestamp` / `getContextTimestamp`.
Closes #16886
https://claude.ai/code/session_013yfLZ8H2qzN2Ya61m3rN3P
Adds three new watch cases and extends the existing one: - `loader-context-dep-no-rerun` now also covers a real dir change after the fix has been kicking in (loader must re-run) and a follow-up unrelated edit (loader must stay cached) — guards against the fix over-suppressing legitimate invalidations. - `loader-file-dep-no-rerun` mirrors the scenario for `addDependency` on a single file, exercising the file-side `_fileTimestamps` cache. - `loader-context-dep-multi` runs two loaders, each with its own `addContextDependency`, and asserts that a change inside one dir re-runs only the matching loader, while an unrelated edit re-runs neither. - `loader-context-dep-add-delete` covers add/delete inside the watched dir and an unrelated edit between them. https://claude.ai/code/session_013yfLZ8H2qzN2Ya61m3rN3P
`addMissingDependency` records files/directories that don't exist yet but
should be watched. When watchpack later populates `_fileTimestamps` with
an `ExistenceOnlyTimeEntry` (`{}`) for such a path, the snapshot create
and check sites used `Boolean(cache)` directly — which is `true` for
`{}` even though the path does not actually exist — and falsely flipped
the `existence` value, invalidating the snapshot.
Both the snapshot CREATE and CHECK paths for `missingExistence` now
treat existence-only entries as a cache miss and re-stat the file,
matching the behavior already applied elsewhere.
Adds:
- `test/watchCases/context/loader-missing-dep/` covering an
`addMissingDependency` loader across an unrelated edit, the missing
path being created, and another unrelated edit.
- Two unit tests in `FileSystemInfo.unittest.js`: snapshot stays valid
when watchpack reports `{}` for a still-missing dep, and is correctly
invalidated when the missing path actually appears.
https://claude.ai/code/session_013yfLZ8H2qzN2Ya61m3rN3P
🦋 Changeset detectedLatest commit: 9bf94d1 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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
|
This PR is packaged and the instant preview is available (cb824e5). Install it locally:
npm i -D webpack@https://pkg.pr.new/webpack@cb824e5
yarn add -D webpack@https://pkg.pr.new/webpack@cb824e5
pnpm add -D webpack@https://pkg.pr.new/webpack@cb824e5 |
Codecov Report❌ Patch coverage is
❌ Your patch check has failed because the patch coverage (77.90%) is below the target coverage (90.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## main #20916 +/- ##
==========================================
- Coverage 91.37% 91.35% -0.02%
==========================================
Files 563 566 +3
Lines 55822 56001 +179
Branches 14788 14866 +78
==========================================
+ Hits 51006 51160 +154
- Misses 4816 4841 +25
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes watch-mode snapshot invalidation when watchpack provides partial timestamp entries (notably existence-only {}), which previously caused context-dependent loaders to spuriously rerun once after the first change. The PR updates FileSystemInfo cache typing/lookup behavior to treat such entries as “no usable info” and fall back to on-disk reads, and adds both unit and end-to-end watch case coverage for the regression.
Changes:
- Extend
FileTimestamp/ContextTimestamphandling to include watchpack existence-only entries and bypass them during cache lookups and snapshot validity checks. - Add
FileSystemInfounit tests covering{}and{ safeTime }watchpack entries for snapshot validity and timestamp getters. - Add new watch-mode end-to-end repro cases for
addContextDependency, plus related dependency-watch scenarios.
Reviewed changes
Copilot reviewed 39 out of 39 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
.changeset/fix-context-dependency-rerun-watch-mode.md |
Patch changeset describing the watchpack existence-only entry fix and user-visible impact. |
lib/FileSystemInfo.js |
Core fix: accept {} in timestamp caches and treat existence-only / insufficient entries as cache misses during snapshotting and validation. |
test/FileSystemInfo.unittest.js |
Adds unit tests to ensure {} / { safeTime } watchpack entries don’t falsely invalidate snapshots and that getters fall back to disk. |
test/watchCases/context/loader-context-dep-add-delete/0/directory/a.txt |
Seed context directory contents for add/delete watch case. |
test/watchCases/context/loader-context-dep-add-delete/0/index.js |
Watch e2e assertions for add/delete inside context dir vs unrelated edits. |
test/watchCases/context/loader-context-dep-add-delete/0/loader.js |
Loader using addContextDependency and directory read to observe dir contents. |
test/watchCases/context/loader-context-dep-add-delete/0/unrelated.js |
Unrelated module used to trigger rebuilds without touching the context dir. |
test/watchCases/context/loader-context-dep-add-delete/1/directory/b.txt |
Step file added to the watched directory to trigger invalidation. |
test/watchCases/context/loader-context-dep-add-delete/2/unrelated.js |
Step-only unrelated edit to ensure no spurious rerun. |
test/watchCases/context/loader-context-dep-add-delete/3/directory/b.txt |
Step deletion marker (DELETE) to remove b.txt and trigger invalidation. |
test/watchCases/context/loader-context-dep-multi/0/dir-a/seed-a.txt |
Seed file for loader A watched directory. |
test/watchCases/context/loader-context-dep-multi/0/dir-b/seed-b.txt |
Seed file for loader B watched directory. |
test/watchCases/context/loader-context-dep-multi/0/index.js |
Watch e2e assertions that only the loader whose context dir changed reruns. |
test/watchCases/context/loader-context-dep-multi/0/loader-a.js |
Loader A: addContextDependency on dir-a. |
test/watchCases/context/loader-context-dep-multi/0/loader-b.js |
Loader B: addContextDependency on dir-b. |
test/watchCases/context/loader-context-dep-multi/0/unrelated.js |
Unrelated module used to trigger rebuilds without touching either watched dir. |
test/watchCases/context/loader-context-dep-multi/1/dir-a/added.txt |
Step change inside dir-a to trigger only loader A rerun. |
test/watchCases/context/loader-context-dep-multi/2/dir-b/added.txt |
Step change inside dir-b to trigger only loader B rerun. |
test/watchCases/context/loader-context-dep-multi/3/unrelated.js |
Step-only unrelated edit to ensure neither loader reruns. |
test/watchCases/context/loader-context-dep-no-rerun/0/directory/a.txt |
Seed file for single watched directory. |
test/watchCases/context/loader-context-dep-no-rerun/0/index.js |
Watch e2e assertions that unrelated edits don’t rerun an addContextDependency loader. |
test/watchCases/context/loader-context-dep-no-rerun/0/loader.js |
Loader: addContextDependency on directory/. |
test/watchCases/context/loader-context-dep-no-rerun/0/unrelated.js |
Unrelated module used to trigger rebuilds without touching the context dir. |
test/watchCases/context/loader-context-dep-no-rerun/1/unrelated.js |
Step-only unrelated edit to ensure loader output is cached. |
test/watchCases/context/loader-context-dep-no-rerun/2/unrelated.js |
Step-only unrelated edit to ensure loader output is cached. |
test/watchCases/context/loader-context-dep-no-rerun/3/directory/b.txt |
Step adds a file inside watched dir to trigger loader invalidation. |
test/watchCases/context/loader-context-dep-no-rerun/4/unrelated.js |
Step-only unrelated edit after real change to ensure loader stays cached. |
test/watchCases/context/loader-file-dep-no-rerun/0/index.js |
Watch e2e assertions for addDependency only rerunning when tracked file changes. |
test/watchCases/context/loader-file-dep-no-rerun/0/loader.js |
Loader: addDependency on a specific file. |
test/watchCases/context/loader-file-dep-no-rerun/0/tracked.txt |
Initial tracked file contents. |
test/watchCases/context/loader-file-dep-no-rerun/0/unrelated.js |
Unrelated module used to trigger rebuilds without touching tracked file. |
test/watchCases/context/loader-file-dep-no-rerun/1/unrelated.js |
Step-only unrelated edit to ensure loader output is cached. |
test/watchCases/context/loader-file-dep-no-rerun/2/tracked.txt |
Step changes tracked file contents to force loader rerun. |
test/watchCases/context/loader-missing-dep/0/index.js |
Watch e2e assertions for addMissingDependency invalidating only when the missing path appears. |
test/watchCases/context/loader-missing-dep/0/loader.js |
Loader: addMissingDependency on a file that appears in a later step. |
test/watchCases/context/loader-missing-dep/0/unrelated.js |
Unrelated module used to trigger rebuilds without touching the missing path. |
test/watchCases/context/loader-missing-dep/1/unrelated.js |
Step-only unrelated edit to ensure loader output is cached while target is still missing. |
test/watchCases/context/loader-missing-dep/2/future.json |
Step creates previously-missing dependency file. |
test/watchCases/context/loader-missing-dep/3/unrelated.js |
Step-only unrelated edit to ensure loader stays cached after the missing file exists. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
2718
to
2723
| if (cacheUsable) { | ||
| this._resolveContextTimestamp( | ||
| /** @type {ContextFileSystemInfoEntry} */ | ||
| (cache), | ||
| (fullCache), | ||
| callback | ||
| ); |
Member
Author
There was a problem hiding this comment.
Fixed in a727318. Updated the callback param JSDoc to ResolvedContextTimestamp and the cast on the snapshot store to ResolvedContextFileSystemInfoEntry | null.
Generated by Claude Code
Comment on lines
2681
to
2693
| const cacheUsable = cache !== undefined && !isExistenceOnly(cache); | ||
| const fullCache = | ||
| /** @type {ContextFileSystemInfoEntry | null | undefined} */ ( | ||
| cacheUsable ? cache : undefined | ||
| ); | ||
| if ( | ||
| cache !== undefined && | ||
| (resolved = getResolvedTimestamp(cache)) !== undefined | ||
| cacheUsable && | ||
| (resolved = getResolvedTimestamp( | ||
| /** @type {ContextFileSystemInfoEntry | null} */ (fullCache) | ||
| )) !== undefined | ||
| ) { | ||
| contextTimestamps.set(path, resolved); | ||
| } else { |
Member
Author
There was a problem hiding this comment.
Good catch — fixed in a727318. The CREATE path now also detects usableCache.timestampHash === undefined (mirroring the cacheLacksHash logic on the CHECK path) and routes those entries through _readFreshContextTimestamp so the snapshot always stores a complete entry. Added a unit test (snapshot creation re-reads disk when cache for a context dir lacks timestampHash) that pre-populates the cache with { safeTime: 1 } and asserts the resulting snapshot has a timestampHash.
Generated by Claude Code
…Hash
Addresses two issues flagged in PR review on the snapshot CREATE path
for context timestamps:
1. The intermediate callback was typed as `FileTimestamp` /
`FileSystemInfoEntry` even though it operates on context entries.
Updated to `ResolvedContextTimestamp` /
`ResolvedContextFileSystemInfoEntry`.
2. `cacheUsable` only filtered out existence-only (`{}`) entries. A
`{ safeTime }` cache entry without `timestampHash` was passed
through `getResolvedTimestamp` and stored directly into the
snapshot, defeating directory-change detection because future
snapshot validity relies on `timestampHash`. The CREATE path now
detects `usableCache.timestampHash === undefined` and routes those
paths through `_readFreshContextTimestamp`, mirroring the CHECK
site.
Also adds a unit test that pre-populates the cache with `{ safeTime }`
and asserts the resulting snapshot stores a complete entry with
`timestampHash`.
https://claude.ai/code/session_013yfLZ8H2qzN2Ya61m3rN3P
Comment on lines
+8
to
+15
| this.fs.stat(target, (_err, stat) => { | ||
| callback( | ||
| null, | ||
| `module.exports = ${JSON.stringify({ | ||
| exists: Boolean(stat), | ||
| random: Math.random() | ||
| })};` | ||
| ); |
Member
Author
There was a problem hiding this comment.
Fixed in 9bf94d1. The loader now forwards non-ENOENT errors via callback(err) and only treats ENOENT (and the success path) as the "missing or stat'd" cases.
Generated by Claude Code
- Regenerate `types.d.ts` after the `FileTimestamp` / `ContextTimestamp` union changes (caught by `lint:special`). - Narrow `getFileTimestamp` callback typing to `FileSystemInfoEntry | "ignore" | null` since existence-only entries are filtered before the callback fires. - Fix the `loader-missing-dep` watch loader to forward non-ENOENT filesystem errors instead of silently treating every error as "missing" (caught by Copilot review on a727318). https://claude.ai/code/session_013yfLZ8H2qzN2Ya61m3rN3P
Contributor
Types CoverageCoverage after merging claude/fix-issue-16886-nV4hO into main will be
Coverage Report
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
aryanraj45
pushed a commit
to aryanraj45/webpack
that referenced
this pull request
May 8, 2026
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.
addFileTimestampsandaddContextTimestampsaccept watchpack-styleExistenceOnlyTimeEntry({}) values, but the cache types previouslynarrowed away that variant via cast. Downstream code then treated
{}as a fully-populated timestamp, so snapshot validity checks compared
"no info" against snapshots that do have a
timestamp/timestampHashand falsely invalidated the snapshot — most visibly causing loaders that
call
addContextDependencyto rerun once after the first change.FileTimestampandContextTimestampnow includeExistenceOnlyTimeEntry,the lying casts in the
add*methods are gone, and every cache lookupuses an
isExistenceOnlycheck to fall back to a fresh on-disk readwhen the cached entry is
{}or otherwise lacks data the snapshotexpects (e.g.
{ safeTime }without atimestampHash).Adds:
test/watchCases/context/loader-context-dep-no-rerun/end-to-endreproduction.
FileSystemInfounit tests coveringaddFileTimestamps/addContextTimestampswith{}and{ safeTime }entries, both forsnapshot validity and for
getFileTimestamp/getContextTimestamp.Closes #16886
https://claude.ai/code/session_013yfLZ8H2qzN2Ya61m3rN3P