Skip to content

Commit b094fca

Browse files
matejchalkmatejchalk
committed
perf(plugin-eslint): run eslint as separate process to prevent exceeding memory
1 parent cef8e7c commit b094fca

File tree

5 files changed

+153
-62
lines changed

5 files changed

+153
-62
lines changed

‎packages/plugin-eslint/mocks/fixtures/todos-app/.eslintrc.js

+1
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
/** @type {import('eslint').ESLint.ConfigData} */
22
module.exports = {
3+
root: true,
34
env: {
45
browser: true,
56
es2021: true,

‎packages/plugin-eslint/src/lib/runner.integration.test.ts

+3-3
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ describe('executeRunner', () => {
2121
let cwdSpy: MockInstance<[], string>;
2222
let platformSpy: MockInstance<[], NodeJS.Platform>;
2323

24-
const createPluginConfig = async (eslintrc: string) => {
24+
const createPluginConfig = async (eslintrc: ESLintTarget['eslintrc']) => {
2525
const patterns = ['src/**/*.js', 'src/**/*.jsx'];
2626
const targets: ESLintTarget[] = [{ eslintrc, patterns }];
2727
const { audits } = await listAuditsAndGroups(targets);
@@ -66,15 +66,15 @@ describe('executeRunner', () => {
6666
});
6767

6868
it('should execute runner with inline config using @code-pushup/eslint-config', async () => {
69-
await createPluginConfig(ESLINTRC_PATH);
69+
await createPluginConfig({ extends: '@code-pushup' });
7070
await executeRunner();
7171

7272
const json = await readJsonFile<AuditOutput[]>(RUNNER_OUTPUT_PATH);
7373
// expect warnings from unicorn/filename-case rule from default config
7474
expect(json).toContainEqual(
7575
expect.objectContaining<Partial<AuditOutput>>({
7676
slug: 'unicorn-filename-case',
77-
displayValue: '5 warnings',
77+
displayValue: expect.stringMatching(/^\d+ warnings?$/),
7878
details: {
7979
issues: expect.arrayContaining<Issue>([
8080
{

‎packages/plugin-eslint/src/lib/runner/lint.ts

+90-33
Original file line numberDiff line numberDiff line change
@@ -1,44 +1,101 @@
1-
import type { Linter } from 'eslint';
2-
import { distinct, toArray } from '@code-pushup/utils';
3-
import { type ESLintTarget } from '../config';
1+
import type { ESLint, Linter } from 'eslint';
2+
import { rm, writeFile } from 'node:fs/promises';
3+
import { platform } from 'node:os';
4+
import { join } from 'node:path';
5+
import { distinct, executeProcess, toArray } from '@code-pushup/utils';
6+
import type { ESLintTarget } from '../config';
47
import { setupESLint } from '../setup';
58
import type { LinterOutput, RuleOptionsPerFile } from './types';
69

710
export async function lint({
811
eslintrc,
912
patterns,
1013
}: ESLintTarget): Promise<LinterOutput> {
14+
const results = await executeLint({ eslintrc, patterns });
15+
const ruleOptionsPerFile = await loadRuleOptionsPerFile(eslintrc, results);
16+
return { results, ruleOptionsPerFile };
17+
}
18+
19+
function executeLint({
20+
eslintrc,
21+
patterns,
22+
}: ESLintTarget): Promise<ESLint.LintResult[]> {
23+
return withConfig(eslintrc, async configPath => {
24+
// running as CLI because ESLint#lintFiles() runs out of memory
25+
const { stdout } = await executeProcess({
26+
command: 'npx',
27+
args: [
28+
'eslint',
29+
`--config=${configPath}`,
30+
'--no-eslintrc',
31+
'--no-error-on-unmatched-pattern',
32+
'--format=json',
33+
...toArray(patterns).map(pattern =>
34+
// globs need to be escaped on Unix
35+
platform() === 'win32' ? pattern : `'${pattern}'`,
36+
),
37+
],
38+
ignoreExitCode: true,
39+
cwd: process.cwd(),
40+
});
41+
42+
return JSON.parse(stdout) as ESLint.LintResult[];
43+
});
44+
}
45+
46+
function loadRuleOptionsPerFile(
47+
eslintrc: ESLintTarget['eslintrc'],
48+
results: ESLint.LintResult[],
49+
): Promise<RuleOptionsPerFile> {
1150
const eslint = setupESLint(eslintrc);
1251

13-
const results = await eslint.lintFiles(patterns);
14-
15-
const ruleOptionsPerFile = await results.reduce(
16-
async (acc, { filePath, messages }) => {
17-
const filesMap = await acc;
18-
const config = (await eslint.calculateConfigForFile(
19-
filePath,
20-
)) as Linter.Config;
21-
const ruleIds = distinct(
22-
messages
23-
.map(({ ruleId }) => ruleId)
24-
.filter((ruleId): ruleId is string => ruleId != null),
25-
);
26-
const rulesMap = Object.fromEntries(
27-
ruleIds.map(ruleId => [
28-
ruleId,
29-
toArray(config.rules?.[ruleId] ?? []).slice(1),
30-
]),
31-
);
32-
return {
33-
...filesMap,
34-
[filePath]: {
35-
...filesMap[filePath],
36-
...rulesMap,
37-
},
38-
};
39-
},
40-
Promise.resolve<RuleOptionsPerFile>({}),
41-
);
52+
return results.reduce(async (acc, { filePath, messages }) => {
53+
const filesMap = await acc;
54+
const config = (await eslint.calculateConfigForFile(
55+
filePath,
56+
)) as Linter.Config;
57+
const ruleIds = distinct(
58+
messages
59+
.map(({ ruleId }) => ruleId)
60+
.filter((ruleId): ruleId is string => ruleId != null),
61+
);
62+
const rulesMap = Object.fromEntries(
63+
ruleIds.map(ruleId => [
64+
ruleId,
65+
toArray(config.rules?.[ruleId] ?? []).slice(1),
66+
]),
67+
);
68+
return {
69+
...filesMap,
70+
[filePath]: {
71+
...filesMap[filePath],
72+
...rulesMap,
73+
},
74+
};
75+
}, Promise.resolve<RuleOptionsPerFile>({}));
76+
}
4277

43-
return { results, ruleOptionsPerFile };
78+
async function withConfig<T>(
79+
eslintrc: ESLintTarget['eslintrc'],
80+
fn: (configPath: string) => Promise<T>,
81+
): Promise<T> {
82+
if (typeof eslintrc === 'string') {
83+
return fn(eslintrc);
84+
}
85+
86+
const configPath = generateTempConfigPath();
87+
await writeFile(configPath, JSON.stringify(eslintrc));
88+
89+
try {
90+
return await fn(configPath);
91+
} finally {
92+
await rm(configPath);
93+
}
94+
}
95+
96+
function generateTempConfigPath(): string {
97+
return join(
98+
process.cwd(),
99+
`.eslintrc.${Math.random().toString().slice(2)}.json`,
100+
);
44101
}

‎packages/plugin-eslint/src/lib/runner/lint.unit.test.ts

+56-25
Original file line numberDiff line numberDiff line change
@@ -1,30 +1,10 @@
11
import { ESLint, Linter } from 'eslint';
2+
import { MEMFS_VOLUME } from '@code-pushup/test-utils';
3+
import { executeProcess } from '@code-pushup/utils';
24
import { ESLintPluginConfig } from '../config';
35
import { lint } from './lint';
46

57
class MockESLint {
6-
lintFiles = vi.fn().mockResolvedValue([
7-
{
8-
filePath: `${process.cwd()}/src/app/app.component.ts`,
9-
messages: [
10-
{ ruleId: 'max-lines' },
11-
{ ruleId: '@typescript-eslint/no-explicit-any' },
12-
{ ruleId: '@typescript-eslint/no-explicit-any' },
13-
],
14-
},
15-
{
16-
filePath: `${process.cwd()}/src/app/app.component.spec.ts`,
17-
messages: [
18-
{ ruleId: 'max-lines' },
19-
{ ruleId: '@typescript-eslint/no-explicit-any' },
20-
],
21-
},
22-
{
23-
filePath: `${process.cwd()}/src/app/pages/settings.component.ts`,
24-
messages: [{ ruleId: 'max-lines' }],
25-
},
26-
] as ESLint.LintResult[]);
27-
288
calculateConfigForFile = vi.fn().mockImplementation(
299
(path: string) =>
3010
({
@@ -50,6 +30,41 @@ vi.mock('eslint', () => ({
5030
}),
5131
}));
5232

33+
vi.mock('@code-pushup/utils', async () => {
34+
const utils = await vi.importActual('@code-pushup/utils');
35+
// eslint-disable-next-line @typescript-eslint/naming-convention
36+
const testUtils: { MEMFS_VOLUME: string } = await vi.importActual(
37+
'@code-pushup/test-utils',
38+
);
39+
const cwd = testUtils.MEMFS_VOLUME;
40+
return {
41+
...utils,
42+
executeProcess: vi.fn().mockResolvedValue({
43+
stdout: JSON.stringify([
44+
{
45+
filePath: `${cwd}/src/app/app.component.ts`,
46+
messages: [
47+
{ ruleId: 'max-lines' },
48+
{ ruleId: '@typescript-eslint/no-explicit-any' },
49+
{ ruleId: '@typescript-eslint/no-explicit-any' },
50+
],
51+
},
52+
{
53+
filePath: `${cwd}/src/app/app.component.spec.ts`,
54+
messages: [
55+
{ ruleId: 'max-lines' },
56+
{ ruleId: '@typescript-eslint/no-explicit-any' },
57+
],
58+
},
59+
{
60+
filePath: `${cwd}/src/app/pages/settings.component.ts`,
61+
messages: [{ ruleId: 'max-lines' }],
62+
},
63+
] as ESLint.LintResult[]),
64+
}),
65+
};
66+
});
67+
5368
describe('lint', () => {
5469
const config: ESLintPluginConfig = {
5570
eslintrc: '.eslintrc.js',
@@ -77,15 +92,31 @@ describe('lint', () => {
7792
});
7893
});
7994

80-
it('should correctly use ESLint Node API', async () => {
95+
it('should correctly use ESLint CLI and Node API', async () => {
8196
await lint(config);
8297
expect(ESLint).toHaveBeenCalledWith<ConstructorParameters<typeof ESLint>>({
8398
overrideConfigFile: '.eslintrc.js',
8499
useEslintrc: false,
85100
errorOnUnmatchedPattern: false,
86101
});
87-
expect(eslint.lintFiles).toHaveBeenCalledTimes(1);
88-
expect(eslint.lintFiles).toHaveBeenCalledWith(['**/*.js']);
102+
103+
expect(executeProcess).toHaveBeenCalledTimes(1);
104+
expect(executeProcess).toHaveBeenCalledWith<
105+
Parameters<typeof executeProcess>
106+
>({
107+
command: 'npx',
108+
args: [
109+
'eslint',
110+
'--config=.eslintrc.js',
111+
'--no-eslintrc',
112+
'--no-error-on-unmatched-pattern',
113+
'--format=json',
114+
expect.stringContaining('**/*.js'), // wraps in quotes on Unix
115+
],
116+
ignoreExitCode: true,
117+
cwd: MEMFS_VOLUME,
118+
});
119+
89120
expect(eslint.calculateConfigForFile).toHaveBeenCalledTimes(3);
90121
expect(eslint.calculateConfigForFile).toHaveBeenCalledWith(
91122
`${process.cwd()}/src/app/app.component.ts`,

‎packages/plugin-eslint/src/lib/runner/transform.ts

+3-1
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,9 @@ export function lintResultsToAudits({
3636
.reduce<Record<string, LintIssue[]>>((acc, issue) => {
3737
const { ruleId, message, filePath } = issue;
3838
if (!ruleId) {
39-
ui().logger.warning(`ESLint core error - ${message}`);
39+
ui().logger.warning(
40+
`ESLint core error - ${message} (file: ${filePath})`,
41+
);
4042
return acc;
4143
}
4244
const options = ruleOptionsPerFile[filePath]?.[ruleId] ?? [];

0 commit comments

Comments
 (0)