Skip to content
Merged
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/one-linter-per-check.md
Original file line number Diff line number Diff line change
@@ -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.
18 changes: 15 additions & 3 deletions src/checks/eslint-worker.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,15 +29,27 @@ 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<ESLintClass>} 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<ESLintInstance>} the eslint instance this worker lints with
*/
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);
})();
Expand All @@ -56,4 +68,4 @@ async function lintFiles(files) {
return eslint.lintFiles(files);
}

export { lintFiles, setup };
export { lintFiles, loadESLintClass, setup };
13 changes: 7 additions & 6 deletions src/checks/eslint.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down Expand Up @@ -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<LintResult[]>) | undefined} */
let applySuppressions;
Expand Down
115 changes: 69 additions & 46 deletions src/checks/stylelint-worker.js
Original file line number Diff line number Diff line change
Expand Up @@ -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<Stylelint>, lintFiles: (files: string | string[]) => Promise<Reported[]> }} 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<StylelintOptions>} linterOptions the options it lints under
* @returns {Linter} the linter those options describe
*/
function createLinter(stylelintPath, linterOptions) {
/** @type {Promise<Stylelint> | null} */
let loading = null;

/** @type {Partial<StylelintOptions>} */
let linterOptions;
/**
* Lazily load stylelint on first use.
* @returns {Promise<Stylelint>} 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<Stylelint> | null} */
let stylelintPromise = null;
return loading;
};

/**
* Lazily load stylelint on first use.
* @returns {Promise<Stylelint>} 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>} 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>} stylelint instance
*/
function getStylelint() {
return own.getStylelint();
}

/**
* @param {string | string[]} files files
* @returns {Promise<Reported[]>} 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 };
15 changes: 7 additions & 8 deletions src/checks/stylelint.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down Expand Up @@ -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,
};
Expand Down
62 changes: 62 additions & 0 deletions test/stylelint/multiple-checks.test.js
Original file line number Diff line number Diff line change
@@ -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<EXPECTED_ANY>} 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,
);
});
});
11 changes: 11 additions & 0 deletions types/checks/eslint-worker.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,17 @@ export type ESLintInstance = InstanceType<ESLintClass>;
* @returns {Promise<LintResult[]>} what eslint found in them
*/
export function lintFiles(files: string[]): Promise<LintResult[]>;
/**
* 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<ESLintClass>} the class to lint through
*/
export function loadESLintClass(
path: string,
flat: boolean,
): Promise<ESLintClass>;
/**
* @param {string} path what names the eslint to load
* @param {ESLintOptions} options the options it is constructed with
Expand Down
23 changes: 22 additions & 1 deletion types/checks/stylelint-worker.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<Stylelint>;
lintFiles: (files: string | string[]) => Promise<Reported[]>;
};
/** @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<Stylelint>, lintFiles: (files: string | string[]) => Promise<Reported[]> }} 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<StylelintOptions>} linterOptions the options it lints under
* @returns {Linter} the linter those options describe
*/
export function createLinter(
stylelintPath: string,
linterOptions: Partial<StylelintOptions>,
): Linter;
/**
* Lazily load stylelint on first use.
* @returns {Promise<Stylelint>} stylelint instance
*/
export function getStylelint(): Promise<Stylelint>;
Expand Down
Loading