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..0142f1d 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. @@ -320,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; } @@ -331,11 +353,26 @@ 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. - 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); @@ -343,14 +380,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 +465,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 +788,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..bd1a2d1 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,128 @@ 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 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")); + + 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(