Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/ts-stop-stale-runs.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"diagnostics-webpack-plugin": minor
---

Stop a TypeScript run that nothing will report any more. A check that `reportAs` keeps off the compilation runs after a watch rebuild. When it is on a worker of its own, it now stops a run of an older rebuild as soon as the next one starts, instead of finishing a check whose report would be dropped. Over 600 files, an edit made during a long check was reported after 2.3–2.6 s instead of 4.2–4.5 s, with about 2–3 CPU-seconds less work. Check adapters can stop such a run too, through an optional `cancel()` on their instance.
8 changes: 7 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -515,7 +515,13 @@ The work is moved out of the rebuild rather than off the machine: an edit
arriving while such a check is still running waits for the rest of it, since a
check holds the thread the build runs on. A report a newer rebuild has already
overtaken is dropped, and a check never has two of its runs going at once — the
next starts when the last has reported.
next starts when the last has reported. A TypeScript check on a worker of its
own (see [`threads`](#threads)) stops such a run as soon as the next rebuild
starts, between one file and the next: what the program got through is kept,
and the next run checks the rest. An edit made while a long run is going is
then reported once its own run is done, without waiting for the rest of the
old one. Over 600 files, an edit made while an earlier one was still being
checked was reported after 2.3–2.6 s instead of 4.2–4.5 s.

#### `ignoreDiagnostics`

Expand Down
50 changes: 40 additions & 10 deletions src/check.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,17 +23,20 @@ import { toPosixPath } from "./utils.js";
/** @typedef {Dependencies & { errors?: DiagnosticError, warnings?: DiagnosticError, outputReport?: OutputReportContent }} Report */
/** @typedef {{ lint: (files: string[]) => void, keep: (files: string[]) => void, report: () => Promise<Report>, detach: (report: Promise<Report>) => void }} Runner */
/** @typedef {Map<string, CheckResult | undefined>} ResultStore */
/** @typedef {{ key: string, run: Promise<Run>, joined: boolean }} SharedRun */

/** @type {WeakMap<Compilation["compiler"], Map<string, ResultStore>>} */
const resultStores = new WeakMap();

/** @type {WeakMap<Compilation["compiler"], Map<string, Promise<void>>>} */
/** @type {WeakMap<Compilation["compiler"], Map<string, { settled: Promise<void>, cancel: () => void }>>} */
const detachedReports = new WeakMap();

// A run of a check that is still going, so that two compilers over the same
// files drive the tool once. It is kept only while it runs: what comes after
// belongs to the store of the compiler that made it.
/** @type {Map<string, Promise<Run>>} */
// belongs to the store of the compiler that made it. A run another compiler
// joined is one that compiler waits for too, so it is never stopped, and a run
// that is stopped is no longer there to join.
/** @type {Map<string, SharedRun>} */
const sharedRuns = new Map();

// What only decides which files are checked, or how what is found is reported.
Expand Down Expand Up @@ -106,6 +109,8 @@ function getResultStore(compilation, check) {
/**
* The report of a run left to finish on its own, taken so that the next run of
* the same check waits for it rather than driving a second tool beside it.
* Nothing prints a report of an older compilation, so a check that can stop
* one is told to.
* @param {Compilation} compilation compilation
* @param {string} check a key unique to the check within the compiler
* @returns {Promise<void> | undefined} the report still to land, if there is one
Expand All @@ -119,7 +124,11 @@ function takeDetachedReport(compilation, check) {

reports.delete(check);

return report;
if (!report) return undefined;

report.cancel();

return report.settled;
}

/**
Expand Down Expand Up @@ -249,6 +258,9 @@ function createCheckRunner(key, { id, name, adapter, options }, compilation) {
const linted = new Set();
// A check that cannot lint fails the same way for every batch it is given.
let failed = false;
// The runs this one started that another compiler could join.
/** @type {SharedRun[]} */
const runs = [];

/**
* What the check answers with, and what it says it read to answer — taken
Expand All @@ -262,7 +274,11 @@ function createCheckRunner(key, { id, name, adapter, options }, compilation) {
const key = sharing && `${sharing}\0${files.toSorted().join("\0")}`;
const running = key ? sharedRuns.get(key) : undefined;

if (running) return running;
if (running) {
running.joined = true;

return running.run;
}

const started = instance.lintFiles(files).then((results) => ({
results,
Expand All @@ -274,8 +290,10 @@ function createCheckRunner(key, { id, name, adapter, options }, compilation) {

if (key) {
const forget = () => sharedRuns.delete(key);
const shared = { key, run: started, joined: false };

sharedRuns.set(key, started);
sharedRuns.set(key, shared);
runs.push(shared);
started.then(forget, forget);
}

Expand Down Expand Up @@ -497,13 +515,25 @@ function createCheckRunner(key, { id, name, adapter, options }, compilation) {
detachedReports.set(compilation.compiler, reports);
}

reports.set(
checkKey,
report.then(
reports.set(checkKey, {
settled: report.then(
() => {},
() => {},
),
);
cancel: () => {
if (runs.some((shared) => shared.joined)) return;

for (const shared of runs) {
if (sharedRuns.get(shared.key) === shared) {
sharedRuns.delete(shared.key);
}
}

pending.then((instance) => {
if (instance && instance.cancel) instance.cancel();
});
},
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Resolved
});
}

return { detach, keep, lint, report };
Expand Down
1 change: 1 addition & 0 deletions src/checks/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ import typescript from "./typescript.js";
* @property {((results: CheckResult[], keep: (message: Message) => boolean) => CheckResult[])=} filterResults keeps what a predicate accepts of every result, which is what `ignore` is applied through
* @property {(formatter?: FormatterOption) => Promise<Format>} getFormatter loads a formatter, falling back to the tool's default one
* @property {() => Promise<void>} cleanup releases whatever the tool holds after a run
* @property {(() => void)=} cancel stops a run of an older compilation that is still going, which nothing reports any more; what it answers with is dropped
*/

/**
Expand Down
105 changes: 64 additions & 41 deletions src/checks/typescript-program.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import { omitPluginOptions } from "../utils.js";
* hands them back, and the program that type checked them.
* What a run of the check answers with: what it found, and what a watcher has
* to follow for it to answer the same way again.
* @typedef {{ diagnostics: Diagnostic[], host: EXPECTED_ANY, files: string[], directories: string[], writes: string[] }} Found
* @typedef {{ diagnostics: Diagnostic[], host: EXPECTED_ANY, files: string[], directories: string[], writes: string[], cancelled?: boolean }} Found
*/

/**
Expand Down Expand Up @@ -400,19 +400,21 @@ function buildSolution(ts, options, configFile, host, unrecoverable, held) {
* every rebuild.
* @param {Held} held what the last compilation left behind, with what its declarations said
* @param {EXPECTED_ANY} program the program built this time
* @param {EXPECTED_ANY=} cancellation what tells a run nobody waits for any more to stop
* @returns {Diagnostic[]} what the declarations of all its files say
*/
function declarationDiagnostics(held, program) {
function declarationDiagnostics(held, program, cancellation) {
// Asked before the semantic diagnostics are: once those are known the
// builder has nothing left that it would check again.
let reached = held.declarations.size === 0;
/** @type {Set<string>} */
const affected = new Set();

for (
let next = held.program.getSemanticDiagnosticsOfNextAffectedFile();
let next =
held.program.getSemanticDiagnosticsOfNextAffectedFile(cancellation);
next;
next = held.program.getSemanticDiagnosticsOfNextAffectedFile()
next = held.program.getSemanticDiagnosticsOfNextAffectedFile(cancellation)
) {
// The whole program, where a change reaches every file.
if (next.affected === program) reached = true;
Expand All @@ -432,7 +434,7 @@ function declarationDiagnostics(held, program) {
file.fileName,
!reached && known && !affected.has(file.fileName)
? known
: program.getDeclarationDiagnostics(file),
: program.getDeclarationDiagnostics(file, cancellation),
);
}

Expand All @@ -448,9 +450,10 @@ function declarationDiagnostics(held, program) {
* @param {TypeScript} ts the loaded TypeScript
* @param {Options} options options
* @param {Held} held what the last compilation left behind
* @param {EXPECTED_ANY=} cancellation what tells a run nobody waits for any more to stop
* @returns {Found} what it found, and what it read to find it
*/
function check(ts, options, held) {
function check(ts, options, held, cancellation) {
const context = String(options.context);
const configFile =
options.configFile ||
Expand Down Expand Up @@ -557,33 +560,63 @@ function check(ts, options, held) {
global: true,
...options.diagnosticOptions,
};
const diagnostics = [
...unrecoverable,
...ts.sortAndDeduplicateDiagnostics([
...program.getConfigFileParsingDiagnostics(),
...program.getOptionsDiagnostics(),
...(kinds.syntactic ? held.program.getSyntacticDiagnostics() : []),
...(kinds.global ? program.getGlobalDiagnostics() : []),
...(kinds.declaration &&
(parsed.options.declaration || parsed.options.composite)
? declarationDiagnostics(held, program)
: []),
...(kinds.semantic ? held.program.getSemanticDiagnostics() : []),
]),
];

// The config file decides which files the program holds, so reading it
// again is what a change to it takes.
const files = [configFile, ...extended, ...parsed.fileNames];
// What `include` covers, which is where a file the program has never held
// can appear.
const directories = Object.keys(parsed.wildcardDirectories || {});
/** @type {string[]} */
const writes = [];
const writes = overrides.noEmit
? []
: [
parsed.options.outDir,
parsed.options.declarationDir,
parsed.options.tsBuildInfoFile,
]
.filter(Boolean)
.map(String);
/** @type {Diagnostic[]} */
let diagnostics;

if (!overrides.noEmit) {
for (const written of [
parsed.options.outDir,
parsed.options.declarationDir,
parsed.options.tsBuildInfoFile,
]) {
if (written) writes.push(String(written));
}
try {
diagnostics = [
...unrecoverable,
...ts.sortAndDeduplicateDiagnostics([
...program.getConfigFileParsingDiagnostics(),
...program.getOptionsDiagnostics(),
...(kinds.syntactic
? held.program.getSyntacticDiagnostics(undefined, cancellation)
: []),
...(kinds.global ? program.getGlobalDiagnostics(cancellation) : []),
...(kinds.declaration &&
(parsed.options.declaration || parsed.options.composite)
? declarationDiagnostics(held, program, cancellation)
: []),
...(kinds.semantic
? held.program.getSemanticDiagnostics(undefined, cancellation)
: []),
]),
];
} catch (err) {
if (!(err instanceof ts.OperationCanceledException)) throw err;

// The builder keeps what it got through and checks the rest next time.
// Which files the declarations were drained of is lost with the run, so
// the next one works all of them out again.
held.declarations = new Map();

return {
cancelled: true,
diagnostics: [],
host,
files,
directories,
writes,
};
}

if (!overrides.noEmit) {
held.program.emit(
undefined,
(
Expand All @@ -601,17 +634,7 @@ function check(ts, options, held) {
if (!held.seen.has(file)) held.files.delete(file);
}

return {
diagnostics,
host,
// The config file decides which files the program holds, so reading it
// again is what a change to it takes.
files: [configFile, ...extended, ...parsed.fileNames],
// What `include` covers, which is where a file the program has never held
// can appear.
directories: Object.keys(parsed.wildcardDirectories || {}),
writes,
};
return { diagnostics, host, files, directories, writes };
}
/**
* What a diagnostic says, as data: a `Diagnostic` holds the source file it was
Expand Down
23 changes: 20 additions & 3 deletions src/checks/typescript-worker.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,9 @@ import { check, describe, getHeld } from "./typescript-program.js";
/** @type {Options} */
let checkOptions;

/** @type {Int32Array} */
let stopped;

/** @type {Promise<TypeScript> | null} */
let loading = null;

Expand All @@ -36,21 +39,35 @@ async function getTypeScript() {

/**
* @param {Options} options the options the check was configured with
* @param {Int32Array} stop the last run the plugin no longer waits for, shared with it
*/
function setup(options) {
function setup(options, stop) {
checkOptions = options;
stopped = stop;
loading = null;
}

/**
* Runs the check and answers with what crosses back: a `Diagnostic` holds the
* source file it was found in, so each one is described and formatted here.
* @param {string[]} files nothing: the program is the config file's
* @param {number} run which run of the check this is
* @returns {Promise<EXPECTED_ANY>} what it found, and what it read to find it
*/
async function lintFiles() {
async function lintFiles(files, run) {
const ts = await getTypeScript();
const held = getHeld(compiler, "worker");
const found = check(ts, checkOptions, held);
// TypeScript asks between one file and the next, which is where a run
// nobody waits for any more stops.
const cancellation = {
isCancellationRequested: () => Atomics.load(stopped, 0) >= run,
throwIfCancellationRequested() {
if (Atomics.load(stopped, 0) >= run) {
throw new ts.OperationCanceledException();
}
},
};
const found = check(ts, checkOptions, held, cancellation);
// Each on its own, so that what is left after `ignoreDiagnostics` can be
// printed without asking the worker again. Each block carries the newline
// TypeScript ends it with, which is what separates one from the next, so
Expand Down
5 changes: 5 additions & 0 deletions src/checks/typescript.js
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,11 @@ async function create({ key, id: entry, options, compilation }) {

return found.diagnostics;
},
// A run of an older compilation is left to finish on the worker while
// webpack builds the next one, which waits for it before checking again.
cancel() {
if (worker) worker.cancel();
},
readFiles() {
return read;
},
Expand Down
Loading
Loading