diff --git a/.changeset/fix-static-watch-ignores-output-path.md b/.changeset/fix-static-watch-ignores-output-path.md new file mode 100644 index 0000000000..f618bc7d5b --- /dev/null +++ b/.changeset/fix-static-watch-ignores-output-path.md @@ -0,0 +1,5 @@ +--- +"webpack-dev-server": patch +--- + +Stop watching the compiler's `output.path` for static file changes, so a build that writes into a static directory no longer reloads the page on its own output. diff --git a/lib/Server.js b/lib/Server.js index 382de1914a..555f84e944 100644 --- a/lib/Server.js +++ b/lib/Server.js @@ -1964,12 +1964,83 @@ class Server { if (watchFiles.length > 0) { for (const item of watchFiles) { if (item.watch) { - await this.watchFiles(item.directory, item.watch); + const ignoreOutputPath = this.getOutputPathMatcher(item.directory); + + await this.watchFiles(item.directory, { + ...item.watch, + ignored: [ + ...(typeof item.watch.ignored === "undefined" + ? [] + : Array.isArray(item.watch.ignored) + ? item.watch.ignored + : [item.watch.ignored]), + ...(ignoreOutputPath ? [ignoreOutputPath] : []), + ], + }); } } } } + /** + * A static directory often contains the compiler's own output, because + * `output.path` sits inside it and `writeToDisk` puts the build there. Every + * build then rewrites those files, and watching them turns one compilation + * into a reload — or, when a plugin copies them back in, into an endless one. + * The compilation already reaches the client through the middleware, so an + * output directory nested in a watched one is never worth watching. + * + * Only a strictly nested output path is excluded. `output.path` defaults to + * `/` under an in-memory filesystem and is routinely left there, and that + * path contains every static directory there is: treating it as output would + * silently stop watching all of them. + * @private + * @param {string} directory the static directory being watched + * @returns {((targetPath: string) => boolean) | undefined} a matcher for paths inside a nested output directory, or nothing to exclude + */ + getOutputPathMatcher(directory) { + const compilers = + /** @type {MultiCompiler} */ + (this.compiler).compilers || [this.compiler]; + // `static.directory` is taken as given and may be relative, while + // `outputPath` and the paths the watcher reports are absolute, so + // everything is resolved before being compared + const resolvedDirectory = path.resolve(directory); + /** + * @param {string} parent the directory to test against + * @param {string} child the path that may sit below it + * @returns {boolean} true when `child` is strictly inside `parent` + */ + const isInside = (parent, child) => { + const relative = path.relative(parent, child); + + return ( + relative !== "" && + relative !== ".." && + !relative.startsWith(`..${path.sep}`) && + !path.isAbsolute(relative) + ); + }; + const outputPaths = compilers + .map((compiler) => compiler.outputPath) + .filter(Boolean) + .map((outputPath) => path.resolve(outputPath)) + .filter((outputPath) => isInside(resolvedDirectory, outputPath)); + + if (outputPaths.length === 0) { + return undefined; + } + + return (targetPath) => { + const resolvedTarget = path.resolve(targetPath); + + return outputPaths.some( + (outputPath) => + resolvedTarget === outputPath || isInside(outputPath, resolvedTarget), + ); + }; + } + /** * @private * @returns {Promise} diff --git a/test/ports-map.js b/test/ports-map.js index d414a52d80..9f2b1076ed 100644 --- a/test/ports-map.js +++ b/test/ports-map.js @@ -81,6 +81,7 @@ const listOfTests = { "api-plugin": 1, "api-plugin-multi": 2, "harness-server-cleanup": 1, + "static-watch-output-path": 3, }; let startPort = 8089; diff --git a/test/server/static-watch-output-path.test.js b/test/server/static-watch-output-path.test.js new file mode 100644 index 0000000000..bb8f433b37 --- /dev/null +++ b/test/server/static-watch-output-path.test.js @@ -0,0 +1,268 @@ +import fsPromises from "node:fs/promises"; +import os from "node:os"; +import path from "node:path"; +import { after, before, describe, it } from "node:test"; +import { expect } from "expect"; +import fs from "graceful-fs"; +import webpack from "webpack"; +import Server from "../../lib/Server.js"; +import portsMap from "../ports-map.js"; + +const [port, rootOutputPort, relativeDirectoryPort] = + portsMap["static-watch-output-path"]; + +// The ignored file is rewritten this many times, this far apart, so that the +// window spans several seconds. A single write could land while chokidar is +// still discovering the nested directory and be lost; a stream of them cannot +// all be. +const REWRITES = 12; +const REWRITE_INTERVAL_MS = 250; + +const sleep = (ms) => + new Promise((resolve) => { + setTimeout(resolve, ms); + }); + +// Rewrites `file` until the watcher reports it, which establishes that the +// watcher is live. It does not establish that every nested path has been +// discovered, so it is only the starting gun — the assertions below do not +// rest on it alone. +const waitUntilWatching = async (reloads, file, timeout = 20000) => { + const started = Date.now(); + + while (!reloads.includes(file)) { + if (Date.now() - started > timeout) { + throw new Error(`the static watcher never reported ${file}`); + } + + await fsPromises.writeFile(file, `warm-up ${Date.now()}`); + await sleep(REWRITE_INTERVAL_MS); + } +}; + +// The watched set fills in as chokidar scans and is empty right after `start()`, +// so it is polled until it has something and then given a moment to finish. +// Paths are resolved because a relative `static.directory` is reported as given. +const watchedDirectories = async (server, timeout = 20000) => { + const collect = () => + server.staticWatchers + .flatMap((watcher) => Object.keys(watcher.getWatched())) + .map((watchedPath) => path.resolve(watchedPath)); + const started = Date.now(); + + while (collect().length === 0) { + if (Date.now() - started > timeout) { + throw new Error("the static watcher never reported a watched directory"); + } + + await sleep(REWRITE_INTERVAL_MS); + } + + await sleep(2000); + + return collect(); +}; + +const waitForReload = (reloads, file, timeout = 10000) => + new Promise((resolve, reject) => { + const started = Date.now(); + const check = () => { + if (reloads.includes(file)) { + resolve(); + return; + } + + if (Date.now() - started > timeout) { + reject(new Error(`timed out waiting for a reload of ${file}`)); + return; + } + + setTimeout(check, 50); + }; + + check(); + }); + +describe("static watching and output.path", () => { + let tempDirectory; + let outputPath; + let insideOutput; + let outsideOutput; + let compiler; + let server; + let reloads; + + before(async () => { + tempDirectory = await fsPromises.mkdtemp( + path.join(os.tmpdir(), "wds-static-output-"), + ); + outputPath = path.join(tempDirectory, "build"); + insideOutput = path.join(outputPath, "emitted.txt"); + outsideOutput = path.join(tempDirectory, "asset.txt"); + + await fsPromises.mkdir(outputPath, { recursive: true }); + // both files must already exist: chokidar reports a new file as `add`, + // and only a rewrite of a known file as the `change` that reloads + await fsPromises.writeFile(insideOutput, "emitted"); + await fsPromises.writeFile(outsideOutput, "asset"); + await fsPromises.writeFile( + path.join(tempDirectory, "entry.js"), + "module.exports = 1;", + ); + + compiler = webpack({ + mode: "development", + context: tempDirectory, + entry: "./entry.js", + output: { path: outputPath }, + infrastructureLogging: { level: "none" }, + stats: "none", + }); + + server = new Server( + { + static: { directory: tempDirectory, watch: true }, + port, + }, + compiler, + ); + + reloads = []; + + const sendMessage = server.sendMessage.bind(server); + + server.sendMessage = (clients, type, data) => { + if (type === "static-changed") { + reloads.push(String(data)); + } + + return sendMessage(clients, type, data); + }; + + await server.start(); + + // chokidar drops events raised before its initial scan finishes, so a write + // sent too early is simply lost. `ready` cannot be awaited from here — the + // watcher is created inside `start()` and may already have emitted it — so + // the control file is rewritten until the watcher answers. + await waitUntilWatching(reloads, outsideOutput); + reloads.length = 0; + }); + + after(async () => { + await server.stop(); + fs.rmSync(tempDirectory, { recursive: true, force: true }); + }); + + it("should not watch the output directory at all", () => { + // the strongest form of the assertion, and the only one that does not + // depend on event timing: chokidar lists what it decided to watch, so a + // predicate that failed to exclude the output directory shows up here even + // if no write ever raced with the scan + const watched = server.staticWatchers.flatMap((watcher) => + Object.keys(watcher.getWatched()), + ); + + expect(watched).toContain(tempDirectory); + expect(watched).not.toContain(outputPath); + }); + + it("should not reload when a file inside output.path is rewritten", async () => { + // the compilation already reaches the client through the middleware, so a + // reload here only fires because the build wrote its own output. Rewriting + // throughout the window rather than once means a single write lost to the + // initial scan cannot hide a broken predicate. + for (let index = 0; index < REWRITES; index++) { + await fsPromises.writeFile(insideOutput, `emitted ${index}`); + await sleep(REWRITE_INTERVAL_MS); + } + + expect(reloads).toHaveLength(0); + }); + + it("should keep watching when output.path contains the static directory", async () => { + // `output.path` defaults to `/` under an in-memory filesystem and is + // routinely left there, and every static directory is inside it. Treating + // that as output would stop watching everything. + const { root } = path.parse(tempDirectory); + const rootOutputCompiler = webpack({ + mode: "development", + context: tempDirectory, + entry: "./entry.js", + output: { path: root }, + infrastructureLogging: { level: "none" }, + stats: "none", + }); + const rootOutputServer = new Server( + { + static: { directory: tempDirectory, watch: true }, + port: rootOutputPort, + }, + rootOutputCompiler, + ); + + /** @type {string[]} */ + const rootOutputReloads = []; + const sendMessage = rootOutputServer.sendMessage.bind(rootOutputServer); + + rootOutputServer.sendMessage = (clients, type, data) => { + if (type === "static-changed") { + rootOutputReloads.push(String(data)); + } + + return sendMessage(clients, type, data); + }; + + await rootOutputServer.start(); + + try { + await waitUntilWatching(rootOutputReloads, outsideOutput); + + expect(rootOutputReloads).toContain(outsideOutput); + } finally { + await rootOutputServer.stop(); + // the first server watches the same directory, so it saw those writes too + reloads.length = 0; + } + }); + + it("should exclude the output path when the static directory is relative", async () => { + // `static.directory` is taken as given, so it can be relative while + // `outputPath` and the watcher's own paths are absolute. Comparing them + // unresolved makes the exclusion quietly match nothing. + const relativeDirectory = path.relative(process.cwd(), tempDirectory); + const relativeCompiler = webpack({ + mode: "development", + context: tempDirectory, + entry: "./entry.js", + output: { path: outputPath }, + infrastructureLogging: { level: "none" }, + stats: "none", + }); + const relativeServer = new Server( + { + static: { directory: relativeDirectory, watch: true }, + port: relativeDirectoryPort, + }, + relativeCompiler, + ); + + await relativeServer.start(); + + try { + const watched = await watchedDirectories(relativeServer); + + expect(watched).toContain(tempDirectory); + expect(watched).not.toContain(outputPath); + } finally { + await relativeServer.stop(); + } + }); + + it("should still reload for the rest of the static directory", async () => { + await fsPromises.writeFile(outsideOutput, "asset again"); + await waitForReload(reloads, outsideOutput); + + expect(reloads).toStrictEqual([outsideOutput]); + }); +}); diff --git a/types/lib/Server.d.ts b/types/lib/Server.d.ts index 16c4696521..36353cf953 100644 --- a/types/lib/Server.d.ts +++ b/types/lib/Server.d.ts @@ -1636,6 +1636,23 @@ declare class Server< * @returns {Promise} */ private setupWatchStaticFiles; + /** + * A static directory often contains the compiler's own output, because + * `output.path` sits inside it and `writeToDisk` puts the build there. Every + * build then rewrites those files, and watching them turns one compilation + * into a reload — or, when a plugin copies them back in, into an endless one. + * The compilation already reaches the client through the middleware, so an + * output directory nested in a watched one is never worth watching. + * + * Only a strictly nested output path is excluded. `output.path` defaults to + * `/` under an in-memory filesystem and is routinely left there, and that + * path contains every static directory there is: treating it as output would + * silently stop watching all of them. + * @private + * @param {string} directory the static directory being watched + * @returns {((targetPath: string) => boolean) | undefined} a matcher for paths inside a nested output directory, or nothing to exclude + */ + private getOutputPathMatcher; /** * @private * @returns {Promise}