Skip to content

Commit 2b40c96

Browse files
alexander-akaitaryanraj45
alexander-akait
and
aryanraj45
authored
fix(watch): purge what a rebuild stops watching (#22646)
* fix(watch): purge what a rebuild stops watching The input file system cache is purged only for the paths the watcher reports, and a change to a path nobody watches is never reported. So a file edited, deleted or created while unwatched, or a directory a loader lists, was read stale for up to 60 s once a rebuild used it again. Each watch call now purges the files, directories and missing paths the previous call watched and this one does not, in one exact purge. Co-authored-by: aryanraj45 <[email protected]> * fix(watch): purge the input cache when watching closes A change made while no watcher runs is never reported, so a watch started again within 60 s built from cached content. --------- Co-authored-by: aryanraj45 <[email protected]>
1 parent b1ae770 commit 2b40c96

37 files changed

Lines changed: 347 additions & 5 deletions

‎.changeset/023-watch-purge-unwatched.md‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"webpack": patch
3+
---
4+
5+
Read files and directories changed while unwatched afresh in watch mode.

‎lib/node/NodeEnvironmentPlugin.js‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,16 @@ class NodeEnvironmentPlugin {
7676
inputFileSystem.purge();
7777
}
7878
});
79+
// Nothing reports a change made while no watcher runs, so a later `watch`
80+
// would build from cached content.
81+
compiler.hooks.watchClose.tap(PLUGIN_NAME, () => {
82+
if (
83+
compiler.inputFileSystem === inputFileSystem &&
84+
inputFileSystem.purge
85+
) {
86+
inputFileSystem.purge();
87+
}
88+
});
7989
compiler.hooks.beforeRun.tap(PLUGIN_NAME, (compiler) => {
8090
if (
8191
compiler.inputFileSystem === inputFileSystem &&

‎lib/node/NodeWatchFileSystem.js‎

Lines changed: 57 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,19 @@ const getCanonicalWatchOptions = (options) => {
7676
return snapshot;
7777
};
7878

79+
/** @typedef {Iterable<string> & { has(item: string): boolean }} Lookup */
80+
81+
/**
82+
* Returns the items as they are when they have `has`, else as a set, which
83+
* also keeps a later change by the caller or a one-shot iterable from mattering.
84+
* @param {Iterable<string>} items items
85+
* @returns {Lookup} the items, as something with `has`
86+
*/
87+
const toLookup = (items) =>
88+
typeof (/** @type {Set<string>} */ (items).has) === "function"
89+
? /** @type {Set<string>} */ (items)
90+
: new Set(items);
91+
7992
class NodeWatchFileSystem {
8093
/**
8194
* Creates an instance of NodeWatchFileSystem.
@@ -91,6 +104,39 @@ class NodeWatchFileSystem {
91104
// `watch` replaces this with a watcher built from its own options
92105
/** @type {Watchpack | null} */
93106
this.watcher = null;
107+
/** @type {{ files: Lookup, directories: Lookup, missing: Lookup } | undefined} */
108+
this._watched = undefined;
109+
}
110+
111+
/**
112+
* Purges what the previous call watched and this one does not: an edit to a
113+
* path nobody watches is never reported, so its cached stat and content would
114+
* be read stale once the path is watched again.
115+
* @param {Lookup} files files to watch
116+
* @param {Lookup} directories directories to watch
117+
* @param {Lookup} missing missing paths to watch
118+
* @returns {void}
119+
*/
120+
_purgeUnwatched(files, directories, missing) {
121+
const previous = this._watched;
122+
this._watched = { files, directories, missing };
123+
const fs = this.inputFileSystem;
124+
if (previous === undefined || !fs || !fs.purge) return;
125+
/** @type {string[]} */
126+
const dropped = [];
127+
for (const list of [
128+
previous.files,
129+
previous.directories,
130+
previous.missing
131+
]) {
132+
for (const item of list) {
133+
if (!files.has(item) && !directories.has(item) && !missing.has(item)) {
134+
dropped.push(item);
135+
}
136+
}
137+
}
138+
// a prefix purge scans the whole cache once per path
139+
if (dropped.length > 0) fs.purge(dropped, { exact: true });
94140
}
95141

96142
/** @type {WatchMethod} */
@@ -124,6 +170,10 @@ class NodeWatchFileSystem {
124170
if (typeof callbackUndelayed !== "function" && callbackUndelayed) {
125171
throw new Error("Invalid arguments: 'callbackUndelayed'");
126172
}
173+
const filesLookup = toLookup(files);
174+
const directoriesLookup = toLookup(directories);
175+
const missingLookup = toLookup(missing);
176+
this._purgeUnwatched(filesLookup, directoriesLookup, missingLookup);
127177
const oldWatcher = this.watcher;
128178
this.watcher = new (getWatchpack())(getCanonicalWatchOptions(options));
129179

@@ -144,9 +194,6 @@ class NodeWatchFileSystem {
144194
}
145195
return { fileTimeInfoEntries, contextTimeInfoEntries };
146196
};
147-
const directoriesSet =
148-
directories instanceof Set ? directories : new Set(directories);
149-
150197
// Watchpack reports a watched directory whenever its contents change, and
151198
// `fs.purge(dir)` matches by prefix, wiping every file's stat cache with it.
152199
// Only its own entry is purged; the batch's file events carry the rest.
@@ -159,7 +206,7 @@ class NodeWatchFileSystem {
159206
if (!fs || !fs.purge) return;
160207
if (changes) {
161208
for (const item of changes) {
162-
if (directoriesSet.has(item)) {
209+
if (directoriesLookup.has(item)) {
163210
fs.purge(item, { exact: true });
164211
} else {
165212
fs.purge(item);
@@ -197,7 +244,12 @@ class NodeWatchFileSystem {
197244
}
198245
);
199246

200-
this.watcher.watch({ files, directories, missing, startTime });
247+
this.watcher.watch({
248+
files: filesLookup,
249+
directories: directoriesLookup,
250+
missing: missingLookup,
251+
startTime
252+
});
201253

202254
if (oldWatcher) {
203255
oldWatcher.close();

‎test/unitCases/NodeWatchFileSystem.unittest.js‎

Lines changed: 109 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -108,6 +108,115 @@ describe("NodeWatchFileSystem", () => {
108108
expect(later.options.ignored).toEqual(["**/mutated"]);
109109
});
110110

111+
it("purges what a later call stops watching", () => {
112+
/** @type {Parameters<import("../../lib/fs/fs").Purge>[]} */
113+
const purges = [];
114+
const wfs = new NodeWatchFileSystem(
115+
/** @type {import("../../lib/fs/fs").InputFileSystem} */ ({
116+
purge: (items, options) => {
117+
purges.push([items, options]);
118+
}
119+
})
120+
);
121+
const [kept, moved, dropped, directory, missing] = [
122+
"a.js",
123+
"b.js",
124+
"c.js",
125+
"directory",
126+
"missing.js"
127+
].map((name) => path.join(fixtures, name));
128+
const start = (
129+
/** @type {string[]} */ files,
130+
/** @type {string[]} */ directories,
131+
/** @type {string[]} */ missingFiles
132+
) =>
133+
watchers.push(
134+
wfs.watch(
135+
files,
136+
directories,
137+
missingFiles,
138+
Date.now(),
139+
{},
140+
() => {},
141+
() => {}
142+
)
143+
);
144+
start([kept, moved, dropped], [directory], [missing]);
145+
expect(purges).toEqual([]);
146+
start([kept], [], [moved]);
147+
expect(purges).toEqual([[[dropped, directory, missing], { exact: true }]]);
148+
start([kept], [], [moved]);
149+
expect(purges).toHaveLength(1);
150+
});
151+
152+
it("purges what an array held when watched, though its caller changes it", () => {
153+
/** @type {Parameters<import("../../lib/fs/fs").Purge>[]} */
154+
const purges = [];
155+
const wfs = new NodeWatchFileSystem(
156+
/** @type {import("../../lib/fs/fs").InputFileSystem} */ ({
157+
purge: (items, options) => {
158+
purges.push([items, options]);
159+
}
160+
})
161+
);
162+
const dropped = path.join(fixtures, "c.js");
163+
const files = [file, dropped];
164+
watchers.push(
165+
wfs.watch(
166+
files,
167+
[],
168+
[],
169+
Date.now(),
170+
{},
171+
() => {},
172+
() => {}
173+
)
174+
);
175+
files.pop();
176+
watchers.push(
177+
wfs.watch(
178+
[file],
179+
[],
180+
[],
181+
Date.now(),
182+
{},
183+
() => {},
184+
() => {}
185+
)
186+
);
187+
expect(purges).toEqual([[[dropped], { exact: true }]]);
188+
});
189+
190+
it("hands watchpack a one-shot iterable it read itself", () => {
191+
const wfs = new NodeWatchFileSystem(
192+
/** @type {import("../../lib/fs/fs").InputFileSystem} */ ({
193+
purge: () => {}
194+
})
195+
);
196+
const start = (
197+
/** @type {Iterable<string>} */ files,
198+
/** @type {Iterable<string>} */ missing
199+
) =>
200+
watchers.push(
201+
wfs.watch(
202+
files,
203+
[],
204+
missing,
205+
Date.now(),
206+
{},
207+
() => {},
208+
() => {}
209+
)
210+
);
211+
start([file], []);
212+
// the second call compares against the first, reading these once
213+
start([file].values(), [].values());
214+
const watcher =
215+
/** @type {InstanceType<import("watchpack")>} */
216+
(wfs.watcher);
217+
expect([...watcher.fileWatchers.keys()]).toEqual([file]);
218+
});
219+
111220
it("leaves an unsupported ignored option to watchpack", () => {
112221
expect(() =>
113222
watch({

‎test/unitCases/WatchClose.unittest.js‎

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
require("../helpers/warmup-webpack");
44

5+
const fs = require("fs");
56
const path = require("path");
67
const testDirectory = path.resolve(__dirname, "..");
78
const expectNoDeprecations = require("../helpers/expectNoDeprecations");
@@ -75,4 +76,54 @@ describe("WatchClose", () => {
7576
expect(num).toBe(1111);
7677
});
7778
});
79+
80+
describe("watching again after close", () => {
81+
const fixturePath = path.join(
82+
testDirectory,
83+
"fixtures",
84+
`temp-watch-close-${Date.now()}`
85+
);
86+
const filePath = path.join(fixturePath, "file.js");
87+
88+
beforeAll(() => {
89+
fs.mkdirSync(fixturePath, { recursive: true });
90+
fs.writeFileSync(filePath, "module.exports = 'before';", "utf8");
91+
});
92+
93+
afterAll(() => {
94+
fs.unlinkSync(filePath);
95+
fs.rmdirSync(fixturePath);
96+
});
97+
98+
it("should read a file edited while closed", async () => {
99+
const webpack = require("../../");
100+
101+
const outputPath = path.join(testDirectory, "js/WatchCloseAgain");
102+
const compiler = webpack({
103+
mode: "development",
104+
entry: filePath,
105+
output: { path: outputPath, filename: "bundle.js" }
106+
});
107+
/**
108+
* @returns {Promise<string>} the emitted bundle of the first build
109+
*/
110+
const build = () =>
111+
new Promise((resolve, reject) => {
112+
const watching = /** @type {import("../../").Watching} */ (
113+
compiler.watch({}, (err) => {
114+
if (err) return reject(err);
115+
watching.close(() =>
116+
resolve(
117+
fs.readFileSync(path.join(outputPath, "bundle.js"), "utf8")
118+
)
119+
);
120+
})
121+
);
122+
});
123+
124+
expect(await build()).toContain("before");
125+
fs.writeFileSync(filePath, "module.exports = 'after';", "utf8");
126+
expect(await build()).toContain("after");
127+
});
128+
});
78129
});

‎test/watchCases/cache/change-directory-while-unwatched/0/ctx/a.js‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
module.exports = "a";

‎test/watchCases/cache/change-directory-while-unwatched/0/index.js‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
import keys from "./sub/keys.js";
2+
import expected from "./sub/expected.js";
3+
4+
it("should list a directory changed while nothing watched it", () => {
5+
expect(keys).toEqual(expected);
6+
});

‎test/watchCases/cache/change-directory-while-unwatched/0/loader.js‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
"use strict";
2+
3+
const path = require("path");
4+
5+
/** @type {import("../../../../../").LoaderDefinition} */
6+
module.exports = function loader() {
7+
const callback = this.async();
8+
const directory = path.resolve(this.rootContext, "ctx");
9+
this.addContextDependency(directory);
10+
this.fs.readdir(directory, (err, names) => {
11+
if (err) return callback(err);
12+
const list = /** @type {string[]} */ (names).map(String).sort();
13+
callback(null, `export default ${JSON.stringify(list)};`);
14+
});
15+
};

‎test/watchCases/cache/change-directory-while-unwatched/0/sub/expected.js‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
export default ["a.js"];

‎test/watchCases/cache/change-directory-while-unwatched/0/sub/keys.js‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
export { default } from "../loader.js!./list.js";

0 commit comments

Comments
 (0)