From 812f6d21abc5981e8d0b7e99c0148b5b199e0dc8 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 29 Oct 2026 10:53:51 +0000 Subject: [PATCH 1/2] perf: tell a file's first change in a watch from what it held, and read TypeScript files in an allowJs project The importers of a file changed while webpack watches are linted again only when what it shows them changes, but that could be told only from a file's second change on: what it showed before its first was never read. What each file a check reading across files covers holds is now kept once the first build of a watch is over, and its first change is told from it. A file written after webpack started reading the file system for that build is not kept, and its first change reaches every importer, as before. What a file shows was read under the project's own compiler options, so in a project setting allowJs, or a rootDir a file is outside of, TypeScript refused to read any file and every edit reached every importer. It is now read under no project option, and only for TypeScript files: a JavaScript file's exports are written as any without a word, so a change to them would not show. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01App18sYbe6Vf6HDCjCXbvf --- .changeset/first-edit-face.md | 5 ++ README.md | 11 +++-- src/import-graph.js | 81 +++++++++++++++++++++++++++--- src/index.js | 54 +++++++++++++++++++- test/import-graph.test.js | 92 +++++++++++++++++++++++++++++++++++ test/incremental.test.js | 63 +++++++++++++++++++++++- 6 files changed, 292 insertions(+), 14 deletions(-) create mode 100644 .changeset/first-edit-face.md diff --git a/.changeset/first-edit-face.md b/.changeset/first-edit-face.md new file mode 100644 index 0000000..09fd9bb --- /dev/null +++ b/.changeset/first-edit-face.md @@ -0,0 +1,5 @@ +--- +"diagnostics-webpack-plugin": patch +--- + +Leave the importers of a file be on its first edit in a watch too when what it shows them does not change, rather than only from its second edit on: what each file held is kept once the first build of the watch is over. In a project whose `tsconfig.json` sets `allowJs`, or a `rootDir` a file is outside of, what a TypeScript file shows its importers could not be read at all, so every edit linted every file importing it again; it is now read without the project's own compiler options. Over three hundred modules under typed linting, the first edit of a function's body took a rebuild from 2.0–2.2 s to 0.27–0.35 s. diff --git a/README.md b/README.md index b277dce..f375dd5 100644 --- a/README.md +++ b/README.md @@ -397,10 +397,13 @@ A file changed while webpack watches reaches its importers only when what it shows them changes: the declarations it emits and what it imports. Edit the body of a function or a comment and only that file is linted again — told apart by `ts.transpileDeclaration` (TypeScript 5.5 and later), which needs no -type checker and so answers only for a file whose exports say their own types, -and only from the second edit of a file on, the first being what it is told -from. Over three hundred modules in a chain, editing one halfway along took a -typed-linting rebuild from 1.1–1.4 s to 0.16–0.2 s. +type checker and so answers only for a TypeScript file whose exports say their +own types. What each file held as the watch started is kept once the first +build is over, so the first edit of a file is told apart too; a file written +after webpack started reading the file system for that build is not kept, and +its first edit reaches every file importing it. Over three hundred modules in a +chain, editing one halfway along took a typed-linting rebuild from 1.1–1.4 s to +0.16–0.2 s, and the first edit of it from 2.0–2.2 s to 0.27–0.35 s. Left out, each check reads it from its tool's configuration: diff --git a/src/import-graph.js b/src/import-graph.js index ffac51b..9e505ea 100644 --- a/src/import-graph.js +++ b/src/import-graph.js @@ -3,6 +3,7 @@ import { createHash } from "node:crypto"; import { statSync } from "node:fs"; +import { readFile, stat } from "node:fs/promises"; import { createRequire } from "node:module"; import { dirname, extname, join, resolve } from "node:path"; import { pathToFileURL } from "node:url"; @@ -42,7 +43,10 @@ import { toPosixPath } from "./utils.js"; * @property {Map} configHashes each config file and what it extends, hashed, as the signatures were last read under * @property {Map} resolutions what each set of compiler options resolved this pass, so that every file in a folder does not look its imports up again * @property {Map} faces what each file that changed showed the files importing it, as the last change left it, hashed + * @property {Map} texts what each file held when the watch started, kept until it first changes so that its first change is told from it + * @property {Promise | undefined} keeping the reading of what the files held when the watch started, which a rebuild waits for * @property {Set | undefined} scripts what a file TypeScript reads ends in, read from it once + * @property {string[] | undefined} typed what a TypeScript file, as against a JavaScript one, ends in, read from it once * @property {EXPECTED_ANY} fallback the options of a directory no config file covers */ @@ -102,7 +106,10 @@ function createGraph(ts) { configHashes: new Map(), resolutions: new Map(), faces: new Map(), + texts: new Map(), + keeping: undefined, scripts: undefined, + typed: undefined, fallback: undefined, }; } @@ -272,11 +279,24 @@ function faceOf(graph, file, text, imports) { return undefined; } + if (!graph.typed) graph.typed = ts.getSupportedExtensions({}).flat(); + + // Only TypeScript says when it cannot write a type without a checker: the + // exports of a JavaScript file are written `any` without a word, and no + // change to them would show. + if ( + !(/** @type {string[]} */ (graph.typed).some((one) => file.endsWith(one))) + ) { + return undefined; + } + const emitted = ts.transpileDeclaration(text, { fileName: file, reportDiagnostics: true, + // Not the project's own options: the old text of a file and the new are + // read alike either way, and some of them — `allowJs`, a `rootDir` the + // file is outside of — make TypeScript refuse to read one at all. compilerOptions: { - ...optionsFor(graph, file), declaration: true, // What an importer's rules read and a project's own emit may drop: a // `@deprecated` in a comment, an `@internal` export. @@ -331,7 +351,12 @@ function affectedFiles(graph, files, changed) { const quiet = new Set(); for (const file of touched) { - const before = graph.files.get(file); + // What a file held before its first change, kept from when the watch + // started, stands in for the face no change has left yet. + const kept = graph.texts.get(file); + const before = + graph.files.get(file) || + (kept === undefined ? undefined : importsOf(graph, file, kept)); // What a file declared for every other one is gone once it changes, as // much as what it declares now is new. @@ -343,14 +368,22 @@ function affectedFiles(graph, files, changed) { if (text === undefined) { graph.faces.delete(file); + graph.texts.delete(file); continue; } const imports = importsOf(graph, file, text); - const was = graph.faces.get(file); - // Only a file that changed is read for it: the first edit of a file is - // taken to reach everything importing it, and the next ones are told from - // what it showed after that one. + let was = graph.faces.get(file); + + if (was === undefined && kept !== undefined && before) { + was = faceOf(graph, file, kept, before); + } + + graph.texts.delete(file); + + // Only a file that changed is read for it: one whose text was not kept + // when the watch started is taken to reach everything importing it on its + // first edit, and the next ones are told from what it showed after that. const face = faceOf(graph, file, text, imports); graph.files.set(file, imports); @@ -420,6 +453,40 @@ function affectedFiles(graph, files, changed) { return new Set([...reached].filter((file) => covered.has(file))); } +/** + * Keeps what each file holds as a watch starts, so that its first change can + * be told from what it showed before it as well as every later one is. A file + * changed since webpack started reading the file system for the build is left + * out: what it holds now may be the change rather than what came before it, + * and its first change reaches everything importing it, as without this. + * @param {Graph} graph the graph + * @param {string[]} files every file the check covers + * @param {number} since when webpack started reading the file system for the build + * @returns {Promise} once every file is kept or left out + */ +async function keepTexts(graph, files, since) { + for (const file of files) { + const known = toPosixPath(resolve(file)); + + if (graph.texts.has(known) || graph.faces.has(known)) continue; + + try { + const { mtimeMs } = await stat(known); + + if (mtimeMs >= since) continue; + + const text = await readFile(known, "utf8"); + + // Written while it was read. + if ((await stat(known)).mtimeMs !== mtimeMs) continue; + + graph.texts.set(known, text); + } catch { + // Gone, which its next change says. + } + } +} + /** * The graph a compiler keeps for one check, read with the TypeScript the * check's project installs — the one its own tooling resolves imports with. @@ -709,4 +776,4 @@ function signatures(graph, files) { return found; } -export { affectedFiles, createGraph, getGraph, hashOf, signatures }; +export { affectedFiles, createGraph, getGraph, hashOf, keepTexts, signatures }; diff --git a/src/index.js b/src/index.js index 3767b89..135af00 100644 --- a/src/index.js +++ b/src/index.js @@ -9,6 +9,7 @@ import createCheckRunner from "./check.js"; import { affectedFiles as affectedFromGraph, getGraph, + keepTexts, } from "./import-graph.js"; import { getOptions, reportedAs, validateOptions } from "./options.js"; import { @@ -136,13 +137,49 @@ async function affectedFiles(compiler, check, files) { check.id, ); - return graph && affectedFromGraph(graph, files, changed); + if (!graph) return undefined; + + // What the files held as the watch started, which a first change is told + // from, is read once the first build is over. + await graph.keeping; + + return affectedFromGraph(graph, files, changed); } catch { // Not knowing is answered by linting every file, as before. return undefined; } } +/** + * Keeps what the files a check reading across files covers hold as a watch + * starts, in the graph it is answered from: the first change of one is then + * told from what it held before, as every later one is, rather than taken to + * reach every file importing it. + * @param {Compiler} compiler compiler + * @param {ResolvedCheck} check the check to keep them for + * @param {string[]} files every file the check covers + * @param {number} since when webpack started reading the file system for the build + * @returns {Promise} once they are kept + */ +async function keepWatchStart(compiler, check, files, since) { + if (check.adapter.affectedFiles || !readsAcrossFiles(compiler, check)) { + return; + } + + // Neither throws: a graph that cannot be had, or a file that cannot be read, + // leaves the first change of a file to reach every file importing it. + const graph = await getGraph( + compiler, + String(check.options.context), + check.id, + ); + + if (!graph) return; + + graph.keeping = keepTexts(graph, files, since); + await graph.keeping; +} + /** * Walks the file system for the files a check wants, and says which of them * webpack has just seen change. @@ -461,6 +498,14 @@ class DiagnosticsWebpackPlugin { generation += 1; const mine = generation; + // The first build of a watch, which webpack started reading the file + // system for at this time: what the files hold by its end is kept. + const watchStart = + compiler.watchMode && + !compiler.modifiedFiles && + !compilation.compiler.isChild() + ? compiler.fsStartTime + : undefined; // Globbing the file system does not depend on the module graph, so a // child compilation would only lint what its parent already did. const enabled = compilation.compiler.isChild() @@ -606,6 +651,7 @@ class DiagnosticsWebpackPlugin { check.collecting = collectFromFileSystem(compiler, check).then( (collected) => { + check.files.push(...collected.lint, ...collected.keep); check.pending.push(...collected.lint); check.kept.push(...collected.keep); check.flush(true); @@ -737,6 +783,12 @@ class DiagnosticsWebpackPlugin { readLast.set(id, { read, missing, directories, writes }); await watch(check, report); + if (watchStart !== undefined) { + afterBuild.push(() => { + keepWatchStart(compiler, check, check.files, watchStart); + }); + } + // `reportAs` has already dropped whatever it reports as `false`, // so what is left only needs putting where it belongs. for (const [results, reported] of /** @type {const} */ ([ diff --git a/test/import-graph.test.js b/test/import-graph.test.js index 042eeb8..820cfdb 100644 --- a/test/import-graph.test.js +++ b/test/import-graph.test.js @@ -15,6 +15,7 @@ import { affectedFiles, createGraph, getGraph, + keepTexts, signatures, } from "../src/import-graph.js"; import { toPosixPath } from "../src/utils.js"; @@ -100,6 +101,97 @@ describe("import graph", () => { ); }); + describe("on its first change of a watch", () => { + // Kept as a build that started reading the file system now would. + const keep = () => keepTexts(graph, all.map(at), Date.now() + 1000); + + it("should leave its importers be once only its body or comments change", async () => { + await keep(); + write("leaf.ts", "// a note\nexport const leaf = 1;\n"); + + assert.deepStrictEqual(affected(all, ["leaf.ts"]), ["leaf.ts"]); + }); + + it("should reach its importers once a type it exports changes", async () => { + await keep(); + write("leaf.ts", 'export const leaf = "1";\n'); + + assert.deepStrictEqual(affected(all, ["leaf.ts"]), [ + "leaf.ts", + "middle.ts", + "top.ts", + ]); + }); + + it("should reach its importers once it declares something global", async () => { + write( + "leaf.ts", + "declare global { const shared: number }\nexport {};\n", + ); + await keep(); + write("leaf.ts", "export const leaf = 1;\n"); + + assert.strictEqual(affected(all, ["leaf.ts"]), undefined); + }); + + it("should reach its importers when it changed after the build started reading it", async () => { + // Written since: what is there may be the change itself. + await keepTexts(graph, all.map(at), 0); + write("leaf.ts", "// a note\nexport const leaf = 1;\n"); + + assert.deepStrictEqual(affected(all, ["leaf.ts"]), [ + "leaf.ts", + "middle.ts", + "top.ts", + ]); + }); + + it("should keep what it held only until it first changes", async () => { + await keep(); + write("leaf.ts", 'export const leaf = "1";\n'); + affected(all, ["leaf.ts"]); + // Kept again, as a second build of the watch would not, it is not + // read over what its first change left. + await keep(); + write("leaf.ts", '// a note\nexport const leaf = "1";\n'); + + assert.deepStrictEqual(affected(all, ["leaf.ts"]), ["leaf.ts"]); + }); + + it("should forget a file that has gone", async () => { + await keep(); + rmSync(at("alone.ts")); + + assert.deepStrictEqual(affected(all, ["alone.ts"]), ["alone.ts"]); + assert.strictEqual(graph.texts.has(toPosixPath(at("alone.ts"))), false); + }); + }); + + it("should leave its importers be in a project that also checks JavaScript", () => { + // Options a declaration of the file alone cannot be read under. + write( + "tsconfig.json", + JSON.stringify({ + compilerOptions: { + strict: true, + moduleResolution: "bundler", + allowJs: true, + checkJs: true, + rootDir: "./src", + }, + }), + ); + + assert.deepStrictEqual( + twice( + "leaf.ts", + "export const leaf: number = 2;\n", + "// a note\nexport const leaf: number = 3;\n", + ), + ["leaf.ts"], + ); + }); + it("should reach its importers once a type it exports changes", () => { assert.deepStrictEqual( twice( diff --git a/test/incremental.test.js b/test/incremental.test.js index d6cbaa8..3cf8188 100644 --- a/test/incremental.test.js +++ b/test/incremental.test.js @@ -168,8 +168,14 @@ describe("incremental", () => { * @param {object} options plugin options * @param {(files: string[]) => void} check what to say of what the rebuild linted * @param {() => void} done done + * @param {() => void} edit what changes once the first build is over */ - const rebuild = (options, check, done) => { + const rebuild = ( + options, + check, + done, + edit = () => writeFileSync(leaf, "const leaf = 2;\n"), + ) => { let first = true; watch = pack("watch", { eslintPath, ...options }).watch({}, (err) => { @@ -179,7 +185,7 @@ describe("incremental", () => { first = false; require(eslintPath)._reset(); require(eslintPath)._readAcrossFiles(true); - writeFileSync(leaf, "const leaf = 2;\n"); + edit(); return; } @@ -190,6 +196,59 @@ describe("incremental", () => { }); }; + it("should leave the importers of a file be on its first change when what they see of it holds", (t, done) => { + // TypeScript, which what a file shows its importers is read from. + const faceLeaf = join( + import.meta.dirname, + "fixtures", + "watch-face-leaf.ts", + ); + const faceEntry = join( + import.meta.dirname, + "fixtures", + "watch-face-entry.ts", + ); + const past = new Date(Date.now() - 60_000); + + writeThree(); + writeFileSync( + faceLeaf, + "export function leaf(): number {\n return 1;\n}\n", + ); + writeFileSync( + faceEntry, + 'import { leaf } from "./watch-face-leaf";\nexport const entry = leaf();\n', + ); + + for (const file of [faceLeaf, faceEntry]) utimesSync(file, past, past); + + const forget = () => { + rmSync(other, { force: true }); + rmSync(faceLeaf, { force: true }); + rmSync(faceEntry, { force: true }); + }; + + rebuild( + { files: ["watch-face-leaf.ts", "watch-face-entry.ts"] }, + (files) => { + forget(); + assert.strictEqual(files.length, 1); + assert.match(files[0], /watch-face-leaf\.ts/u); + }, + done, + // Its body only: what the entry sees of it is the same. Made once what + // the files held as the watch started has been read, which a change + // made before then would be read as. + () => + setTimeout(() => { + writeFileSync( + faceLeaf, + "export function leaf(): number {\n return 2;\n}\n", + ); + }, 1000), + ); + }); + it("should lint again what a change reaches among the files it globs", (t, done) => { writeThree(); rebuild( From d0dde41e5dcdebacdf02e9710b375af95b88bb44 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 29 Oct 2026 11:16:36 +0000 Subject: [PATCH 2/2] fix: forget what the files a change linted every file for held A rebuild that lints every file, because a file that changed declares something global or a config file changed, stopped before reading the other files that changed, so what each of them held or showed before stayed in the graph. An edit putting one of them back as it was then left its importers be, though they were last linted against what it held in between. What the files that changed held is now forgotten on both paths. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01App18sYbe6Vf6HDCjCXbvf --- src/import-graph.js | 14 +++++++++++++- test/import-graph.test.js | 31 +++++++++++++++++++++++++++++++ 2 files changed, 44 insertions(+), 1 deletion(-) diff --git a/src/import-graph.js b/src/import-graph.js index 9e505ea..0142f1d 100644 --- a/src/import-graph.js +++ b/src/import-graph.js @@ -340,6 +340,8 @@ function affectedFiles(graph, files, changed) { graph.configs.clear(); graph.options.clear(); graph.faces.clear(); + // What the files that changed held is not what was linted last any more. + for (const file of touched) graph.texts.delete(file); return undefined; } @@ -360,7 +362,17 @@ function affectedFiles(graph, files, changed) { // What a file declared for every other one is gone once it changes, as // much as what it declares now is new. - if (before && (before.global || before.unresolved)) return undefined; + if (before && (before.global || before.unresolved)) { + // Every file is linted again, against what each one that changed holds + // now: what any of them held or showed before says nothing any more. + for (const other of touched) { + graph.files.delete(other); + graph.faces.delete(other); + graph.texts.delete(other); + } + + return undefined; + } graph.files.delete(file); diff --git a/test/import-graph.test.js b/test/import-graph.test.js index 820cfdb..bd1a2d1 100644 --- a/test/import-graph.test.js +++ b/test/import-graph.test.js @@ -158,6 +158,37 @@ describe("import graph", () => { assert.deepStrictEqual(affected(all, ["leaf.ts"]), ["leaf.ts"]); }); + it("should forget what a file held once a change linted every file", async () => { + write( + "global.ts", + "declare global { const shared: number }\nexport {};\n", + ); + await keepTexts( + graph, + [...all, "global.ts"].map(at), + Date.now() + 1000, + ); + // Linted again with every file: the global one changed alongside. + write( + "global.ts", + "declare global { const shared: string }\nexport {};\n", + ); + write("leaf.ts", 'export const leaf = "1";\n'); + assert.strictEqual( + affected([...all, "global.ts"], ["global.ts", "leaf.ts"]), + undefined, + ); + // Back as it was before the watch, which is not what its importers + // were last linted against. + write("leaf.ts", "export const leaf = 1;\n"); + + assert.deepStrictEqual(affected(all, ["leaf.ts"]), [ + "leaf.ts", + "middle.ts", + "top.ts", + ]); + }); + it("should forget a file that has gone", async () => { await keep(); rmSync(at("alone.ts"));