diff --git a/.changeset/one-linter-per-check.md b/.changeset/one-linter-per-check.md new file mode 100644 index 0000000..a333c77 --- /dev/null +++ b/.changeset/one-linter-per-check.md @@ -0,0 +1,5 @@ +--- +"diagnostics-webpack-plugin": patch +--- + +Load a check's linter for that check rather than once per module, so two Stylelint entries no longer lint under one another's options. diff --git a/src/checks/eslint-worker.js b/src/checks/eslint-worker.js index c4bad10..b46eb87 100644 --- a/src/checks/eslint-worker.js +++ b/src/checks/eslint-worker.js @@ -29,6 +29,19 @@ function setup(path, options, flat) { useFlatConfig = flat; } +/** + * The `ESLint` class the given eslint answers with, which is what says its + * version and applies its fixes as well as what lints. + * @param {string} path what names the eslint to load + * @param {boolean} flat whether to load it in flat mode + * @returns {Promise} the class to lint through + */ +async function loadESLintClass(path, flat) { + const eslintModule = await importFrom(path); + + return eslintModule.loadESLint({ useFlatConfig: flat }); +} + /** * Loads eslint once per worker, on the first file it is given. * @returns {Promise} the eslint instance this worker lints with @@ -36,8 +49,7 @@ function setup(path, options, flat) { function getESLint() { if (!pending) { pending = (async () => { - const eslintModule = await importFrom(specifier); - const ESLint = await eslintModule.loadESLint({ useFlatConfig }); + const ESLint = await loadESLintClass(specifier, useFlatConfig); return new ESLint(eslintOptions); })(); @@ -56,4 +68,4 @@ async function lintFiles(files) { return eslint.lintFiles(files); } -export { lintFiles, setup }; +export { lintFiles, loadESLintClass, setup }; diff --git a/src/checks/eslint.js b/src/checks/eslint.js index 9c6af76..57535a6 100644 --- a/src/checks/eslint.js +++ b/src/checks/eslint.js @@ -6,7 +6,9 @@ import { isAbsolute, join, resolve } from "node:path"; import { fileURLToPath, pathToFileURL } from "node:url"; import { countThreads, createPool } from "../threads.js"; -import { importFrom, omitPluginOptions } from "../utils.js"; +import { omitPluginOptions } from "../utils.js"; + +import { loadESLintClass } from "./eslint-worker.js"; const nodeRequire = createRequire(import.meta.url); @@ -224,12 +226,11 @@ async function create({ options, compilation }) { const fix = Boolean(eslintOptions.fix); const specifier = options.eslintPath || "eslint"; - const eslintModule = await importFrom(specifier); - /** @type {ESLintClass} */ - const ESLint = await eslintModule.loadESLint({ - useFlatConfig: options.configType === "flat", - }); + const ESLint = await loadESLintClass( + specifier, + options.configType === "flat", + ); /** @type {((results: LintResult[]) => Promise) | undefined} */ let applySuppressions; diff --git a/src/checks/stylelint-worker.js b/src/checks/stylelint-worker.js index 6cc8cae..629e1a4 100644 --- a/src/checks/stylelint-worker.js +++ b/src/checks/stylelint-worker.js @@ -5,69 +5,92 @@ import { importFrom } from "../utils.js"; /** @typedef {import("./stylelint.js").StylelintOptions} StylelintOptions */ /** @typedef {import("./stylelint.js").Stylelint} Stylelint */ /** @typedef {import("../options.js").CheckOptions} Options */ +/** @typedef {{ getStylelint: () => Promise, lintFiles: (files: string | string[]) => Promise }} Linter */ -/** @type {string} */ -let stylelintPath = "stylelint"; +/** + * A stylelint loaded under one set of options. A worker holds the one it was + * started with; the plugin's own thread holds one per check, which is what + * keeps two checks of the same tool out of each other's options. + * @param {string} stylelintPath what names the stylelint to load + * @param {Partial} linterOptions the options it lints under + * @returns {Linter} the linter those options describe + */ +function createLinter(stylelintPath, linterOptions) { + /** @type {Promise | null} */ + let loading = null; -/** @type {Partial} */ -let linterOptions; + /** + * Lazily load stylelint on first use. + * @returns {Promise} stylelint instance + */ + const getStylelint = () => { + if (!loading) { + loading = (async () => { + const mod = await importFrom(stylelintPath); + // A `stylelintPath` may name a CommonJS module, which has no default export + return mod.default || mod; + })(); + } -/** @type {Promise | null} */ -let stylelintPromise = null; + return loading; + }; -/** - * Lazily load stylelint on first use. - * @returns {Promise} stylelint instance - */ -async function getStylelint() { - if (!stylelintPromise) { - stylelintPromise = (async () => { - const mod = await importFrom(stylelintPath); - // A `stylelintPath` may name a CommonJS module, which has no default export - return mod.default || mod; - })(); - } + return { + getStylelint, + async lintFiles(files) { + const stylelint = await getStylelint(); + const { results, ruleMetadata } = await stylelint.lint({ + ...linterOptions, + files, + quietDeprecationWarnings: true, + }); - return stylelintPromise; + // Reset result to work with worker + return results.map((result) => ({ + source: result.source, + errored: result.errored, + ignored: result.ignored, + warnings: result.warnings, + deprecations: result.deprecations, + invalidOptionWarnings: result.invalidOptionWarnings, + parseErrors: result.parseErrors, + // What a formatter looks a warning's rule up in. The postcss result it + // otherwise hangs off cannot cross a worker, and this is the same object + // on every result of a batch, so it crosses once. + ruleMetadata, + })); + }, + }; } +// The one this worker was started for, which is a thread of its own. +/** @type {Linter} */ +let own; + /** * @param {Options} options the worker options * @param {Partial} stylelintOptions the stylelint options */ function setup(options, stylelintOptions) { - stylelintPath = options.stylelintPath || "stylelint"; - linterOptions = stylelintOptions; - // Reset cached stylelint in case path changed - stylelintPromise = null; + own = createLinter( + String(options.stylelintPath || "stylelint"), + stylelintOptions, + ); +} + +/** + * @returns {Promise} stylelint instance + */ +function getStylelint() { + return own.getStylelint(); } /** * @param {string | string[]} files files * @returns {Promise} results */ -async function lintFiles(files) { - const stylelint = await getStylelint(); - const { results, ruleMetadata } = await stylelint.lint({ - ...linterOptions, - files, - quietDeprecationWarnings: true, - }); - - // Reset result to work with worker - return results.map((result) => ({ - source: result.source, - errored: result.errored, - ignored: result.ignored, - warnings: result.warnings, - deprecations: result.deprecations, - invalidOptionWarnings: result.invalidOptionWarnings, - parseErrors: result.parseErrors, - // What a formatter looks a warning's rule up in. The postcss result it - // otherwise hangs off cannot cross a worker, and this is the same object - // on every result of a batch, so it crosses once. - ruleMetadata, - })); +function lintFiles(files) { + return own.lintFiles(files); } -export { getStylelint, lintFiles, setup }; +export { createLinter, getStylelint, lintFiles, setup }; diff --git a/src/checks/stylelint.js b/src/checks/stylelint.js index 344b5f0..eb8fef1 100644 --- a/src/checks/stylelint.js +++ b/src/checks/stylelint.js @@ -11,11 +11,7 @@ import { parseFiles, } from "../utils.js"; -import { - getStylelint as getStylelintInstance, - lintFiles, - setup, -} from "./stylelint-worker.js"; +import { createLinter } from "./stylelint-worker.js"; const nodeRequire = createRequire(import.meta.url); @@ -90,11 +86,14 @@ function getStylelintOptions(options) { * @returns {Loaded} loaded stylelint */ function loadStylelint(options) { - setup(options, getStylelintOptions(options)); + const linter = createLinter( + String(options.stylelintPath || "stylelint"), + getStylelintOptions(options), + ); return { - getStylelint: getStylelintInstance, - lintFiles, + getStylelint: linter.getStylelint, + lintFiles: linter.lintFiles, cleanup: async () => {}, threads: 1, }; diff --git a/test/stylelint/multiple-checks.test.js b/test/stylelint/multiple-checks.test.js new file mode 100644 index 0000000..177cb38 --- /dev/null +++ b/test/stylelint/multiple-checks.test.js @@ -0,0 +1,62 @@ +import assert from "node:assert/strict"; +import { join } from "node:path"; +import { describe, it } from "node:test"; + +import webpack from "webpack"; + +import DiagnosticsPlugin from "../../src/index.js"; + +const testDir = import.meta.dirname; +const shared = { cache: false, threads: false }; + +/** + * @param {EXPECTED_ANY[]} checks the entries to run + * @returns {Promise} what the build reported + */ +function reported(checks) { + const compiler = webpack({ + context: join(testDir, "fixtures", "error"), + mode: "development", + entry: "./index", + output: { path: join(testDir, "outputs", "multiple-checks") }, + plugins: [new DiagnosticsPlugin({ checks })], + }); + + return new Promise((resolve, reject) => { + compiler.run((err, stats) => { + if (err) reject(err); + else compiler.close(() => resolve(stats)); + }); + }); +} + +// The entry that reports, and one configured to report nothing at all. +const reporting = { + use: "stylelint", + ...shared, + configFile: join(testDir, ".stylelintrc"), +}; +const silent = { + use: "stylelint", + ...shared, + config: { rules: {} }, + customSyntax: "postcss-scss", +}; + +describe("multiple checks", () => { + it("should lint under an entry's own options rather than another's", async () => { + const alone = await reported([reporting]); + + assert.strictEqual(alone.compilation.errors.length, 1); + + // Each entry loads the tool for itself, so the options of the one written + // last are not what the one before it lints under. + const together = await reported([reporting, silent]); + + assert.strictEqual(together.compilation.errors.length, 1); + assert.strictEqual( + together.compilation.errors[0].message, + alone.compilation.errors[0].message, + ); + }); +}); diff --git a/types/checks/eslint-worker.d.ts b/types/checks/eslint-worker.d.ts index 3d1a1b0..cf199dc 100644 --- a/types/checks/eslint-worker.d.ts +++ b/types/checks/eslint-worker.d.ts @@ -7,6 +7,17 @@ export type ESLintInstance = InstanceType; * @returns {Promise} what eslint found in them */ export function lintFiles(files: string[]): Promise; +/** + * The `ESLint` class the given eslint answers with, which is what says its + * version and applies its fixes as well as what lints. + * @param {string} path what names the eslint to load + * @param {boolean} flat whether to load it in flat mode + * @returns {Promise} the class to lint through + */ +export function loadESLintClass( + path: string, + flat: boolean, +): Promise; /** * @param {string} path what names the eslint to load * @param {ESLintOptions} options the options it is constructed with diff --git a/types/checks/stylelint-worker.d.ts b/types/checks/stylelint-worker.d.ts index db55001..b86da50 100644 --- a/types/checks/stylelint-worker.d.ts +++ b/types/checks/stylelint-worker.d.ts @@ -3,8 +3,29 @@ export type Reported = import("./stylelint.js").Reported; export type StylelintOptions = import("./stylelint.js").StylelintOptions; export type Stylelint = import("./stylelint.js").Stylelint; export type Options = import("../options.js").CheckOptions; +export type Linter = { + getStylelint: () => Promise; + lintFiles: (files: string | string[]) => Promise; +}; +/** @typedef {import("./stylelint.js").LintResult} LintResult */ +/** @typedef {import("./stylelint.js").Reported} Reported */ +/** @typedef {import("./stylelint.js").StylelintOptions} StylelintOptions */ +/** @typedef {import("./stylelint.js").Stylelint} Stylelint */ +/** @typedef {import("../options.js").CheckOptions} Options */ +/** @typedef {{ getStylelint: () => Promise, lintFiles: (files: string | string[]) => Promise }} Linter */ +/** + * A stylelint loaded under one set of options. A worker holds the one it was + * started with; the plugin's own thread holds one per check, which is what + * keeps two checks of the same tool out of each other's options. + * @param {string} stylelintPath what names the stylelint to load + * @param {Partial} linterOptions the options it lints under + * @returns {Linter} the linter those options describe + */ +export function createLinter( + stylelintPath: string, + linterOptions: Partial, +): Linter; /** - * Lazily load stylelint on first use. * @returns {Promise} stylelint instance */ export function getStylelint(): Promise;