What is the problem you're trying to solve?
When Stylelint is used as a language server, or any other long-running Node.js API consumer, there's no way to cancel a stylelint.lint() call that's already in progress. This means that if a user edits a document while a lint is running, the old lint continues running to completion even though its results are going to be thrown away.
In stylelint/vscode-stylelint#914, I'm adding an AbortController-based cancellation system to the extension's worker processes. When a document is re-edited, or a config change triggers re-validation, or so on, the in-flight worker request is aborted immediately so the worker can start handling the new request sooner. But the cancellation can only take effect around the stylelint.lint() call, that is, I can check before and after calling it, but can't interrupt anything that's happening inside lint() itself. The worker still has to wait for lint() to return before it can move on.
This means the entire rule execution phase runs even when we already know the result is stale. For multi-file linting via files, it's even worse given the volume of work with multiple files involved.
What solution would you like to see?
An optional signal property on LinterOptions, accepting a standard AbortSignal:
interface LinterOptions {
// ...
signal?: AbortSignal;
}
Using it, either from the language server or any other consumer, would look like this:
const controller = new AbortController();
// If the document changes before this resolves, call controller.abort() to cancel the lint.
const result = await stylelint.lint({
code: document.getText(),
codeFilename: filePath,
signal: controller.signal,
});
There are a few good places to check signal.aborted that I can think of.
| Location |
Granularity |
Notes |
standalone.mjs, before each file in the files path |
Per-file |
Check before calling lintSource() inside the .map() callback |
lintSource.mjs, before config resolution, before parsing, before rule execution, after rule execution |
Per-phase |
|
lintPostcssResult.mjs, after each rule settles |
Per-rule(ish) |
Only catches aborts between async rules |
The per-phase checks are straightforward, we just add if (signal?.aborted) throw signal.reason; guards at the boundaries of each phase of execution.
The per-rule checkpoint is more interesting because of how lintPostcssResult works. The rule execution loop synchronously enqueues all rules into performRules and then runs them with Promise.all(performRules). Each rule's promise could be wrapped with a post-completion signal check:
const performRules = ruleEntries.map(([ruleName, ruleFunction]) => {
return Promise.all(postcssRoots.map(runRule)).then((result) => {
if (signal?.aborted) {
throw signal.reason;
}
return result;
});
});
return Promise.all(performRules);
For synchronous rules, all the .then() callbacks drain in one microtask batch after all rules run, so the signal check is effectively the same as a single check after Promise.all. For async rules, there's now get a cancellation checkpoint after each rule finishes running, which is the best I can think to do without making rule execution serial.
Potentially, we could allow rules to opt in to more fine-grained cancellation by accepting the signal as an argument and checking it at their leisure. However, I think this is a larger change that is out of scope for what I'm proposing here, and potentially may be too much granularity for most use cases.
Some notes
When fix is used with files, fixed output is written to disk per-file as each one completes. If the signal is aborted mid-run, some files would already be written with fixes applied while the rest would still have their original content. However, this is the same behavior you'd get today by interrupting the CLI with Ctrl+C, so we're not really introducing any fundamentally new risk here. That said, it's probably worth mentioning in the API docs that aborting a run with fix set may leave files in a partially-fixed state. In any case, for the primary use case motivating this feature, there are are rarely if ever any disk writes happening, so it's a non-issue.
This change would be minimal, just requiring a small handful of if statements sprinkled in some parts of our codebase, alongside the .then() wrapper for rule execution promises. We don't need to restructure any control flow or fundamentally change how anything works, nor do we introduce any breaking changes. We're also not trailblazing any new patterns here, AbortController is widely used across Node.js core APIs and the broader ecosystem for exactly this kind of use case. So overall, this feels like an easy win with very little to practically no maintenance burden attached to it.
I'd be happy to put together a PR for this if there's interest. Certainly coming from the language server side, I'm very interested in having this feature here in Stylelint.
What is the problem you're trying to solve?
When Stylelint is used as a language server, or any other long-running Node.js API consumer, there's no way to cancel a
stylelint.lint()call that's already in progress. This means that if a user edits a document while a lint is running, the old lint continues running to completion even though its results are going to be thrown away.In stylelint/vscode-stylelint#914, I'm adding an
AbortController-based cancellation system to the extension's worker processes. When a document is re-edited, or a config change triggers re-validation, or so on, the in-flight worker request is aborted immediately so the worker can start handling the new request sooner. But the cancellation can only take effect around thestylelint.lint()call, that is, I can check before and after calling it, but can't interrupt anything that's happening insidelint()itself. The worker still has to wait forlint()to return before it can move on.This means the entire rule execution phase runs even when we already know the result is stale. For multi-file linting via
files, it's even worse given the volume of work with multiple files involved.What solution would you like to see?
An optional
signalproperty onLinterOptions, accepting a standardAbortSignal:Using it, either from the language server or any other consumer, would look like this:
There are a few good places to check
signal.abortedthat I can think of.standalone.mjs, before each file in thefilespathlintSource()inside the.map()callbacklintSource.mjs, before config resolution, before parsing, before rule execution, after rule executionlintPostcssResult.mjs, after each rule settlesThe per-phase checks are straightforward, we just add
if (signal?.aborted) throw signal.reason;guards at the boundaries of each phase of execution.The per-rule checkpoint is more interesting because of how
lintPostcssResultworks. The rule execution loop synchronously enqueues all rules intoperformRulesand then runs them withPromise.all(performRules). Each rule's promise could be wrapped with a post-completion signal check:For synchronous rules, all the
.then()callbacks drain in one microtask batch after all rules run, so the signal check is effectively the same as a single check afterPromise.all. For async rules, there's now get a cancellation checkpoint after each rule finishes running, which is the best I can think to do without making rule execution serial.Potentially, we could allow rules to opt in to more fine-grained cancellation by accepting the signal as an argument and checking it at their leisure. However, I think this is a larger change that is out of scope for what I'm proposing here, and potentially may be too much granularity for most use cases.
Some notes
When
fixis used withfiles, fixed output is written to disk per-file as each one completes. If the signal is aborted mid-run, some files would already be written with fixes applied while the rest would still have their original content. However, this is the same behavior you'd get today by interrupting the CLI with Ctrl+C, so we're not really introducing any fundamentally new risk here. That said, it's probably worth mentioning in the API docs that aborting a run withfixset may leave files in a partially-fixed state. In any case, for the primary use case motivating this feature, there are are rarely if ever any disk writes happening, so it's a non-issue.This change would be minimal, just requiring a small handful of
ifstatements sprinkled in some parts of our codebase, alongside the.then()wrapper for rule execution promises. We don't need to restructure any control flow or fundamentally change how anything works, nor do we introduce any breaking changes. We're also not trailblazing any new patterns here,AbortControlleris widely used across Node.js core APIs and the broader ecosystem for exactly this kind of use case. So overall, this feels like an easy win with very little to practically no maintenance burden attached to it.I'd be happy to put together a PR for this if there's interest. Certainly coming from the language server side, I'm very interested in having this feature here in Stylelint.