Skip to content

Commit

Permalink
perf: fix lazy loading of core rules (#15606)
Browse files Browse the repository at this point in the history
* perf: fix lazy loading of core rules

* add test
  • Loading branch information
mdjermanovic authored Feb 17, 2022
1 parent 3fc9196 commit 39a2fb3
Show file tree
Hide file tree
Showing 6 changed files with 123 additions and 2 deletions.
9 changes: 7 additions & 2 deletions lib/config/flat-config-array.js
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
module.exports = {
root: true,
rules: {
semi: 2
}
};
1 change: 1 addition & 0 deletions tests/fixtures/lazy-loading-rules/foo.js
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
/* content is not necessary */
42 changes: 42 additions & 0 deletions tests/lib/eslint/eslint.js
Original file line number Diff line number Diff line change
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"));
}
});
});
});
});

0 comments on commit 39a2fb3

Please sign in to comment.