Skip to content

Commit

Permalink
perf(plugin-eslint): run eslint as separate process to prevent exceed…
Browse files Browse the repository at this point in the history
…ing memory
  • Loading branch information
matejchalk committed Apr 29, 2024
1 parent 10dd3c6 commit c25b367
Show file tree
Hide file tree
Showing 6 changed files with 156 additions and 79 deletions.
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
/** @type {import('eslint').ESLint.ConfigData} */
module.exports = {
root: true,
env: {
browser: true,
es2021: true,
Expand Down
25 changes: 6 additions & 19 deletions packages/plugin-eslint/src/lib/runner.integration.test.ts
Original file line number Diff line number Diff line change
@@ -1,17 +1,13 @@
import { ESLint } from 'eslint';
import { rm, writeFile } from 'node:fs/promises';
import os from 'node:os';
import { dirname, join } from 'node:path';
import { fileURLToPath } from 'node:url';
import { MockInstance, describe, expect, it } from 'vitest';
import type { AuditOutput, AuditOutputs, Issue } from '@code-pushup/models';
import { osAgnosticAuditOutputs } from '@code-pushup/test-utils';
import { ensureDirectoryExists, readJsonFile } from '@code-pushup/utils';
import { readJsonFile } from '@code-pushup/utils';
import type { ESLintTarget } from './config';
import { listAuditsAndGroups } from './meta';
import {
ESLINTRC_PATH,
PLUGIN_CONFIG_PATH,
RUNNER_OUTPUT_PATH,
createRunnerConfig,
executeRunner,
Expand All @@ -21,7 +17,7 @@ describe('executeRunner', () => {
let cwdSpy: MockInstance<[], string>;
let platformSpy: MockInstance<[], NodeJS.Platform>;

const createPluginConfig = async (eslintrc: string) => {
const createPluginConfig = async (eslintrc: ESLintTarget['eslintrc']) => {
const patterns = ['src/**/*.js', 'src/**/*.jsx'];
const targets: ESLintTarget[] = [{ eslintrc, patterns }];
const { audits } = await listAuditsAndGroups(targets);
Expand All @@ -37,24 +33,15 @@ describe('executeRunner', () => {
'todos-app',
);

beforeAll(async () => {
beforeAll(() => {
cwdSpy = vi.spyOn(process, 'cwd').mockReturnValue(appDir);
// Windows does not require additional quotation marks for globs
platformSpy = vi.spyOn(os, 'platform').mockReturnValue('win32');

const config: ESLint.ConfigData = {
extends: '@code-pushup',
};
await ensureDirectoryExists(dirname(ESLINTRC_PATH));
await writeFile(ESLINTRC_PATH, JSON.stringify(config));
});

afterAll(async () => {
afterAll(() => {
cwdSpy.mockRestore();
platformSpy.mockRestore();

await rm(ESLINTRC_PATH, { force: true });
await rm(PLUGIN_CONFIG_PATH, { force: true });
});

it('should execute ESLint and create audit results for React application', async () => {
Expand All @@ -66,15 +53,15 @@ describe('executeRunner', () => {
});

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

const json = await readJsonFile<AuditOutput[]>(RUNNER_OUTPUT_PATH);
// expect warnings from unicorn/filename-case rule from default config
expect(json).toContainEqual(
expect.objectContaining<Partial<AuditOutput>>({
slug: 'unicorn-filename-case',
displayValue: '5 warnings',
displayValue: expect.stringMatching(/^\d+ warnings?$/),
details: {
issues: expect.arrayContaining<Issue>([
{
Expand Down
1 change: 0 additions & 1 deletion packages/plugin-eslint/src/lib/runner/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ import type { LinterOutput } from './types';

export const WORKDIR = pluginWorkDir('eslint');
export const RUNNER_OUTPUT_PATH = join(WORKDIR, 'runner-output.json');
export const ESLINTRC_PATH = join(process.cwd(), WORKDIR, '.eslintrc.json');
export const PLUGIN_CONFIG_PATH = join(
process.cwd(),
WORKDIR,
Expand Down
123 changes: 90 additions & 33 deletions packages/plugin-eslint/src/lib/runner/lint.ts
Original file line number Diff line number Diff line change
@@ -1,44 +1,101 @@
import type { Linter } from 'eslint';
import { distinct, toArray } from '@code-pushup/utils';
import { type ESLintTarget } from '../config';
import type { ESLint, Linter } from 'eslint';
import { rm, writeFile } from 'node:fs/promises';
import { platform } from 'node:os';
import { join } from 'node:path';
import { distinct, executeProcess, toArray } from '@code-pushup/utils';
import type { ESLintTarget } from '../config';
import { setupESLint } from '../setup';
import type { LinterOutput, RuleOptionsPerFile } from './types';

export async function lint({
eslintrc,
patterns,
}: ESLintTarget): Promise<LinterOutput> {
const results = await executeLint({ eslintrc, patterns });
const ruleOptionsPerFile = await loadRuleOptionsPerFile(eslintrc, results);
return { results, ruleOptionsPerFile };
}

function executeLint({
eslintrc,
patterns,
}: ESLintTarget): Promise<ESLint.LintResult[]> {
return withConfig(eslintrc, async configPath => {
// running as CLI because ESLint#lintFiles() runs out of memory
const { stdout } = await executeProcess({
command: 'npx',
args: [
'eslint',
`--config=${configPath}`,
'--no-eslintrc',
'--no-error-on-unmatched-pattern',
'--format=json',
...toArray(patterns).map(pattern =>
// globs need to be escaped on Unix
platform() === 'win32' ? pattern : `'${pattern}'`,
),
],
ignoreExitCode: true,
cwd: process.cwd(),
});

return JSON.parse(stdout) as ESLint.LintResult[];
});
}

function loadRuleOptionsPerFile(
eslintrc: ESLintTarget['eslintrc'],
results: ESLint.LintResult[],
): Promise<RuleOptionsPerFile> {
const eslint = setupESLint(eslintrc);

const results = await eslint.lintFiles(patterns);

const ruleOptionsPerFile = await results.reduce(
async (acc, { filePath, messages }) => {
const filesMap = await acc;
const config = (await eslint.calculateConfigForFile(
filePath,
)) as Linter.Config;
const ruleIds = distinct(
messages
.map(({ ruleId }) => ruleId)
.filter((ruleId): ruleId is string => ruleId != null),
);
const rulesMap = Object.fromEntries(
ruleIds.map(ruleId => [
ruleId,
toArray(config.rules?.[ruleId] ?? []).slice(1),
]),
);
return {
...filesMap,
[filePath]: {
...filesMap[filePath],
...rulesMap,
},
};
},
Promise.resolve<RuleOptionsPerFile>({}),
);
return results.reduce(async (acc, { filePath, messages }) => {
const filesMap = await acc;
const config = (await eslint.calculateConfigForFile(
filePath,
)) as Linter.Config;
const ruleIds = distinct(
messages
.map(({ ruleId }) => ruleId)
.filter((ruleId): ruleId is string => ruleId != null),
);
const rulesMap = Object.fromEntries(
ruleIds.map(ruleId => [
ruleId,
toArray(config.rules?.[ruleId] ?? []).slice(1),
]),
);
return {
...filesMap,
[filePath]: {
...filesMap[filePath],
...rulesMap,
},
};
}, Promise.resolve<RuleOptionsPerFile>({}));
}

return { results, ruleOptionsPerFile };
async function withConfig<T>(
eslintrc: ESLintTarget['eslintrc'],
fn: (configPath: string) => Promise<T>,
): Promise<T> {
if (typeof eslintrc === 'string') {
return fn(eslintrc);
}

const configPath = generateTempConfigPath();
await writeFile(configPath, JSON.stringify(eslintrc));

try {
return await fn(configPath);
} finally {
await rm(configPath);
}
}

function generateTempConfigPath(): string {
return join(
process.cwd(),
`.eslintrc.${Math.random().toString().slice(2)}.json`,
);
}
81 changes: 56 additions & 25 deletions packages/plugin-eslint/src/lib/runner/lint.unit.test.ts
Original file line number Diff line number Diff line change
@@ -1,30 +1,10 @@
import { ESLint, Linter } from 'eslint';
import { MEMFS_VOLUME } from '@code-pushup/test-utils';
import { executeProcess } from '@code-pushup/utils';
import { ESLintPluginConfig } from '../config';
import { lint } from './lint';

class MockESLint {
lintFiles = vi.fn().mockResolvedValue([
{
filePath: `${process.cwd()}/src/app/app.component.ts`,
messages: [
{ ruleId: 'max-lines' },
{ ruleId: '@typescript-eslint/no-explicit-any' },
{ ruleId: '@typescript-eslint/no-explicit-any' },
],
},
{
filePath: `${process.cwd()}/src/app/app.component.spec.ts`,
messages: [
{ ruleId: 'max-lines' },
{ ruleId: '@typescript-eslint/no-explicit-any' },
],
},
{
filePath: `${process.cwd()}/src/app/pages/settings.component.ts`,
messages: [{ ruleId: 'max-lines' }],
},
] as ESLint.LintResult[]);

calculateConfigForFile = vi.fn().mockImplementation(
(path: string) =>
({
Expand All @@ -50,6 +30,41 @@ vi.mock('eslint', () => ({
}),
}));

vi.mock('@code-pushup/utils', async () => {
const utils = await vi.importActual('@code-pushup/utils');
// eslint-disable-next-line @typescript-eslint/naming-convention
const testUtils: { MEMFS_VOLUME: string } = await vi.importActual(
'@code-pushup/test-utils',
);
const cwd = testUtils.MEMFS_VOLUME;
return {
...utils,
executeProcess: vi.fn().mockResolvedValue({
stdout: JSON.stringify([
{
filePath: `${cwd}/src/app/app.component.ts`,
messages: [
{ ruleId: 'max-lines' },
{ ruleId: '@typescript-eslint/no-explicit-any' },
{ ruleId: '@typescript-eslint/no-explicit-any' },
],
},
{
filePath: `${cwd}/src/app/app.component.spec.ts`,
messages: [
{ ruleId: 'max-lines' },
{ ruleId: '@typescript-eslint/no-explicit-any' },
],
},
{
filePath: `${cwd}/src/app/pages/settings.component.ts`,
messages: [{ ruleId: 'max-lines' }],
},
] as ESLint.LintResult[]),
}),
};
});

describe('lint', () => {
const config: ESLintPluginConfig = {
eslintrc: '.eslintrc.js',
Expand Down Expand Up @@ -77,15 +92,31 @@ describe('lint', () => {
});
});

it('should correctly use ESLint Node API', async () => {
it('should correctly use ESLint CLI and Node API', async () => {
await lint(config);
expect(ESLint).toHaveBeenCalledWith<ConstructorParameters<typeof ESLint>>({
overrideConfigFile: '.eslintrc.js',
useEslintrc: false,
errorOnUnmatchedPattern: false,
});
expect(eslint.lintFiles).toHaveBeenCalledTimes(1);
expect(eslint.lintFiles).toHaveBeenCalledWith(['**/*.js']);

expect(executeProcess).toHaveBeenCalledTimes(1);
expect(executeProcess).toHaveBeenCalledWith<
Parameters<typeof executeProcess>
>({
command: 'npx',
args: [
'eslint',
'--config=.eslintrc.js',
'--no-eslintrc',
'--no-error-on-unmatched-pattern',
'--format=json',
expect.stringContaining('**/*.js'), // wraps in quotes on Unix
],
ignoreExitCode: true,
cwd: MEMFS_VOLUME,
});

expect(eslint.calculateConfigForFile).toHaveBeenCalledTimes(3);
expect(eslint.calculateConfigForFile).toHaveBeenCalledWith(
`${process.cwd()}/src/app/app.component.ts`,
Expand Down
4 changes: 3 additions & 1 deletion packages/plugin-eslint/src/lib/runner/transform.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,9 @@ export function lintResultsToAudits({
.reduce<Record<string, LintIssue[]>>((acc, issue) => {
const { ruleId, message, filePath } = issue;
if (!ruleId) {
ui().logger.warning(`ESLint core error - ${message}`);
ui().logger.warning(
`ESLint core error - ${message} (file: ${filePath})`,
);
return acc;
}
const options = ruleOptionsPerFile[filePath]?.[ruleId] ?? [];
Expand Down

0 comments on commit c25b367

Please sign in to comment.