Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

perf: fix lazy loading of core rules #15606

Merged
merged 2 commits into from Feb 17, 2022
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
9 changes: 7 additions & 2 deletions lib/config/flat-config-array.js
Expand Up @@ -14,7 +14,6 @@ const { flatConfigSchema } = require("./flat-config-schema");
const { RuleValidator } = require("./rule-validator");
const { defaultConfig } = require("./default-config");
const recommendedConfig = require("../../conf/eslint-recommended");
const allConfig = require("../../conf/eslint-all");

//-----------------------------------------------------------------------------
// Helpers
Expand Down Expand Up @@ -79,7 +78,13 @@ class FlatConfigArray extends ConfigArray {
}

if (config === "eslint:all") {
return allConfig;

/*
* Load `eslint-all.js` here instead of at the top level to avoid loading all rule modules
* when it isn't necessary. `eslint-all.js` reads `meta` of rule objects to filter out deprecated ones,
* so requiring `eslint-all.js` module loads all rule modules as a consequence.
*/
return require("../../conf/eslint-all");
}

return config;
Expand Down
1 change: 1 addition & 0 deletions package.json
Expand Up @@ -122,6 +122,7 @@
"node-polyfill-webpack-plugin": "^1.0.3",
"npm-license": "^0.3.3",
"nyc": "^15.0.1",
"pirates": "^4.0.5",
"progress": "^2.0.3",
"proxyquire": "^2.0.1",
"puppeteer": "^9.1.1",
Expand Down
66 changes: 66 additions & 0 deletions tests/_utils/test-lazy-loading-rules.js
@@ -0,0 +1,66 @@
/**
* @fileoverview Tests lazy-loading of core rules
* @author Milos Djermanovic
*/

/*
* This module should be run as a child process, with `fork()`,
* because it is important to run this test with a separate, clean Node process
* in order to add hooks before any of the ESLint modules is loaded.
*/

"use strict";

const path = require("path");
const assert = require("assert");
const { addHook } = require("pirates");

const {
dir: rulesDirectoryPath,
name: rulesDirectoryIndexFilename
} = path.parse(require.resolve("../../lib/rules"));

// Show full stack trace. The default 10 is usually not enough to find the root cause of this problem.
Error.stackTraceLimit = Infinity;

const [cwd, pattern, usedRulesCommaSeparated] = process.argv.slice(2);

assert.ok(cwd, "cwd argument isn't provided");
assert.ok(pattern, "pattern argument isn't provided");
assert.ok(usedRulesCommaSeparated, "used rules argument isn't provided");

const usedRules = usedRulesCommaSeparated.split(",");

// `require()` hook
addHook(
(_code, filename) => {
throw new Error(`Unexpected attempt to load unused rule ${filename}`);
},
{

// returns `true` if the hook (the function passed in as the first argument) should be called for this filename
matcher(filename) {
const { dir, name } = path.parse(filename);

if (dir === rulesDirectoryPath && ![rulesDirectoryIndexFilename, ...usedRules].includes(name)) {
return true;
}

return false;
}

}
);

/*
* Everything related to loading any ESLint modules should be in this IIFE
*/
(async () => {
const { ESLint } = require("../..");
const eslint = new ESLint({ cwd });

await eslint.lintFiles([pattern]);
})().catch(({ message, stack }) => {
process.send({ message, stack });
process.exit(1);
});
6 changes: 6 additions & 0 deletions tests/fixtures/lazy-loading-rules/.eslintrc.js
@@ -0,0 +1,6 @@
module.exports = {
root: true,
rules: {
semi: 2
}
};
1 change: 1 addition & 0 deletions tests/fixtures/lazy-loading-rules/foo.js
@@ -0,0 +1 @@
/* content is not necessary */
42 changes: 42 additions & 0 deletions tests/lib/eslint/eslint.js
Expand Up @@ -27,6 +27,7 @@ const {
const hash = require("../../../lib/cli-engine/hash");
const { unIndent, createCustomTeardown } = require("../../_utils");
const coreRules = require("../../../lib/rules");
const childProcess = require("child_process");

//------------------------------------------------------------------------------
// Tests
Expand Down Expand Up @@ -6694,4 +6695,45 @@ describe("ESLint", () => {
});
});
});

describe("loading rules", () => {
it("should not load unused core rules", done => {
let calledDone = false;

const cwd = getFixturePath("lazy-loading-rules");
const pattern = "foo.js";
const usedRules = ["semi"];

const forkedProcess = childProcess.fork(
path.join(__dirname, "../../_utils/test-lazy-loading-rules.js"),
[cwd, pattern, String(usedRules)]
);

// this is an error message
forkedProcess.on("message", ({ message, stack }) => {
if (calledDone) {
return;
}
calledDone = true;

const error = new Error(message);

error.stack = stack;
done(error);
});

forkedProcess.on("exit", exitCode => {
if (calledDone) {
return;
}
calledDone = true;

if (exitCode === 0) {
done();
} else {
done(new Error("Forked process exited with a non-zero exit code"));
}
});
});
});
});