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(plugin-eslint): lint nx projects in separate processes #651

Merged
merged 3 commits into from
Apr 29, 2024
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
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
12 changes: 8 additions & 4 deletions packages/plugin-eslint/src/lib/config.ts
Original file line number Diff line number Diff line change
@@ -1,26 +1,30 @@
import type { ESLint } from 'eslint';
import { type ZodType, z } from 'zod';
import { toArray } from '@code-pushup/utils';

Check warning on line 3 in packages/plugin-eslint/src/lib/config.ts

View workflow job for this annotation

GitHub Actions / Code PushUp

<↗> Code coverage | Line coverage

Line 3 is not covered in any test case.

export const eslintPluginConfigSchema = z.object({
export const eslintTargetSchema = z.object({
eslintrc: z.union(
[
z.string({ description: 'Path to ESLint config file' }),
z.record(z.string(), z.unknown(), {
description: 'ESLint config object',
}) as ZodType<ESLint.ConfigData>,
],
{ description: 'ESLint config as file path or inline object' },
),
patterns: z.union([z.string(), z.array(z.string()).min(1)], {
description:
'Lint target files. May contain file paths, directory paths or glob patterns',
}),
});

Check warning on line 19 in packages/plugin-eslint/src/lib/config.ts

View workflow job for this annotation

GitHub Actions / Code PushUp

<↗> Code coverage | Line coverage

Lines 10-19 are not covered in any test case.
export type ESLintTarget = z.infer<typeof eslintTargetSchema>;

export type ESLintPluginConfig = z.infer<typeof eslintPluginConfigSchema>;
export const eslintPluginConfigSchema = z
.union([eslintTargetSchema, z.array(eslintTargetSchema).min(1)])
.transform(toArray);
export type ESLintPluginConfig = z.input<typeof eslintPluginConfigSchema>;

export type ESLintPluginRunnerConfig = {
eslintrc: string;
targets: ESLintTarget[];
slugs: string[];

Check warning on line 29 in packages/plugin-eslint/src/lib/config.ts

View workflow job for this annotation

GitHub Actions / Code PushUp

<↗> Code coverage | Line coverage

Lines 20-29 are not covered in any test case.
patterns: string[];
};

Check warning on line 30 in packages/plugin-eslint/src/lib/config.ts

View workflow job for this annotation

GitHub Actions / Code PushUp

<↗> Code coverage | Line coverage

Line 30 is not covered in any test case.
24 changes: 4 additions & 20 deletions packages/plugin-eslint/src/lib/eslint-plugin.ts
Original file line number Diff line number Diff line change
@@ -1,71 +1,55 @@
import { mkdir, writeFile } from 'node:fs/promises';
import { dirname, join } from 'node:path';
import { fileURLToPath } from 'node:url';
import { PluginConfig } from '@code-pushup/models';
import { name, version } from '../../package.json';
import { ESLintPluginConfig, eslintPluginConfigSchema } from './config';
import { listAuditsAndGroups } from './meta';
import { ESLINTRC_PATH, createRunnerConfig } from './runner';
import { setupESLint } from './setup';
import { createRunnerConfig } from './runner';

/**

Check warning on line 9 in packages/plugin-eslint/src/lib/eslint-plugin.ts

View workflow job for this annotation

GitHub Actions / Code PushUp

<↗> Code coverage | Line coverage

Lines 6-9 are not covered in any test case.
* Instantiates Code PushUp ESLint plugin for use in core config.
*
* @example
* import eslintPlugin from '@code-pushup/eslint-plugin'
*
* export default {
* // ... core config ...
* plugins: [
* // ... other plugins ...
* await eslintPlugin({

Check warning on line 19 in packages/plugin-eslint/src/lib/eslint-plugin.ts

View workflow job for this annotation

GitHub Actions / Code PushUp

<↗> Code coverage | Line coverage

Lines 10-19 are not covered in any test case.
* eslintrc: '.eslintrc.json',
* patterns: ['src', 'test/*.spec.js']
* })
* ]
* }
*
* @param config Configuration options.
* @returns Plugin configuration as a promise.
*/
export async function eslintPlugin(

Check warning on line 29 in packages/plugin-eslint/src/lib/eslint-plugin.ts

View workflow job for this annotation

GitHub Actions / Code PushUp

<↗> Code coverage | Line coverage

Lines 20-29 are not covered in any test case.
config: ESLintPluginConfig,
): Promise<PluginConfig> {
const { eslintrc, patterns } = eslintPluginConfigSchema.parse(config);
const targets = eslintPluginConfigSchema.parse(config);

const eslint = setupESLint(eslintrc);

const { audits, groups } = await listAuditsAndGroups(eslint, patterns);

// save inline config to file so runner can access it later
if (typeof eslintrc !== 'string') {
await mkdir(dirname(ESLINTRC_PATH), { recursive: true });
await writeFile(ESLINTRC_PATH, JSON.stringify(eslintrc));
}
const eslintrcPath = typeof eslintrc === 'string' ? eslintrc : ESLINTRC_PATH;
const { audits, groups } = await listAuditsAndGroups(targets);

const runnerScriptPath = join(
fileURLToPath(dirname(import.meta.url)),
'bin.js',
);

Check warning on line 39 in packages/plugin-eslint/src/lib/eslint-plugin.ts

View workflow job for this annotation

GitHub Actions / Code PushUp

<↗> Code coverage | Line coverage

Lines 30-39 are not covered in any test case.

return {
slug: 'eslint',
title: 'ESLint',
icon: 'eslint',
description: 'Official Code PushUp ESLint plugin',
docsUrl: 'https://www.npmjs.com/package/@code-pushup/eslint-plugin',
packageName: name,
version,

Check warning on line 49 in packages/plugin-eslint/src/lib/eslint-plugin.ts

View workflow job for this annotation

GitHub Actions / Code PushUp

<↗> Code coverage | Line coverage

Lines 40-49 are not covered in any test case.
audits,
groups,

runner: await createRunnerConfig(
runnerScriptPath,
audits,
eslintrcPath,
patterns,
),
runner: await createRunnerConfig(runnerScriptPath, audits, targets),
};
}

Check warning on line 55 in packages/plugin-eslint/src/lib/eslint-plugin.ts

View workflow job for this annotation

GitHub Actions / Code PushUp

<↗> Code coverage | Line coverage

Lines 50-55 are not covered in any test case.
7 changes: 3 additions & 4 deletions packages/plugin-eslint/src/lib/meta/index.ts
Original file line number Diff line number Diff line change
@@ -1,14 +1,13 @@
import type { ESLint } from 'eslint';
import type { Audit, Group } from '@code-pushup/models';
import type { ESLintTarget } from '../config';
import { groupsFromRuleCategories, groupsFromRuleTypes } from './groups';
import { listRules } from './rules';
import { ruleToAudit } from './transform';

export async function listAuditsAndGroups(
eslint: ESLint,
patterns: string | string[],
targets: ESLintTarget[],
): Promise<{ audits: Audit[]; groups: Group[] }> {
const rules = await listRules(eslint, patterns);
const rules = await listRules(targets);

const audits = rules.map(ruleToAudit);

Expand Down
79 changes: 51 additions & 28 deletions packages/plugin-eslint/src/lib/meta/rules.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
import type { ESLint, Linter, Rule } from 'eslint';
import { distinct, toArray, ui } from '@code-pushup/utils';
import type { ESLintTarget } from '../config';
import { setupESLint } from '../setup';
import { jsonHash } from './hash';

export type RuleData = {
Expand All @@ -8,10 +10,23 @@
options: unknown[] | undefined;
};

export async function listRules(
type RulesMap = Record<string, Record<string, RuleData>>;

export async function listRules(targets: ESLintTarget[]): Promise<RuleData[]> {
const rulesMap = await targets.reduce(async (acc, { eslintrc, patterns }) => {
const eslint = setupESLint(eslintrc);
const prev = await acc;
const curr = await loadRulesMap(eslint, patterns);
return mergeRulesMaps(prev, curr);
}, Promise.resolve<RulesMap>({}));

return Object.values(rulesMap).flatMap<RuleData>(Object.values);
}

async function loadRulesMap(
eslint: ESLint,
patterns: string | string[],
): Promise<RuleData[]> {
): Promise<RulesMap> {
const configs = await toArray(patterns).reduce(
async (acc, pattern) => [
...(await acc),
Expand All @@ -31,35 +46,43 @@
} as ESLint.LintResult,
]);

const rulesMap = configs
return configs
.flatMap(config => Object.entries(config.rules ?? {}))
.filter(([, ruleEntry]) => ruleEntry != null && !isRuleOff(ruleEntry))
.reduce<Record<string, Record<string, RuleData>>>(
(acc, [ruleId, ruleEntry]) => {
const meta = rulesMeta[ruleId];
if (!meta) {
ui().logger.warning(`Metadata not found for ESLint rule ${ruleId}`);
return acc;
}
const options = toArray(ruleEntry).slice(1);
const optionsHash = jsonHash(options);
const ruleData: RuleData = {
ruleId,
meta,
options,
};
return {
...acc,
[ruleId]: {
...acc[ruleId],
[optionsHash]: ruleData,
},
};
},
{},
);
.reduce<RulesMap>((acc, [ruleId, ruleEntry]) => {
const meta = rulesMeta[ruleId];
if (!meta) {

Check failure on line 54 in packages/plugin-eslint/src/lib/meta/rules.ts

View workflow job for this annotation

GitHub Actions / Code PushUp

<↗> Code coverage | Branch coverage

1st branch is not taken in any test case.

Check failure on line 54 in packages/plugin-eslint/src/lib/meta/rules.ts

View workflow job for this annotation

GitHub Actions / Code PushUp

<↗> Code coverage | Branch coverage

1st branch is not taken in any test case.
ui().logger.warning(`Metadata not found for ESLint rule ${ruleId}`);
return acc;
}
const options = toArray(ruleEntry).slice(1);
const optionsHash = jsonHash(options);
const ruleData: RuleData = {
ruleId,
meta,
options,
};
return {
...acc,
[ruleId]: {
...acc[ruleId],
[optionsHash]: ruleData,
},
};
}, {});
}

return Object.values(rulesMap).flatMap<RuleData>(Object.values);
function mergeRulesMaps(prev: RulesMap, curr: RulesMap): RulesMap {
return Object.entries(curr).reduce(
(acc, [ruleId, ruleVariants]) => ({
...acc,
[ruleId]: {
...acc[ruleId],
...ruleVariants,
},
}),
prev,
);
}

function isRuleOff(entry: Linter.RuleEntry<unknown[]>): boolean {
Expand Down
30 changes: 12 additions & 18 deletions packages/plugin-eslint/src/lib/meta/rules.unit.test.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { ESLint } from 'eslint';
import { dirname, join } from 'node:path';
import { fileURLToPath } from 'node:url';
import type { MockInstance } from 'vitest';
import type { ESLintTarget } from '../config';
import { RuleData, listRules, parseRuleId } from './rules';

describe('listRules', () => {
Expand All @@ -28,22 +28,19 @@ describe('listRules', () => {
const appRootDir = join(fixturesDir, 'todos-app');
const eslintrc = join(appRootDir, '.eslintrc.js');

const eslint = new ESLint({
useEslintrc: false,
baseConfig: { extends: eslintrc },
});
const patterns = ['src/**/*.js', 'src/**/*.jsx'];
const targets: ESLintTarget[] = [{ eslintrc, patterns }];

beforeAll(() => {
cwdSpy.mockReturnValue(appRootDir);
});

it('should list expected number of rules', async () => {
await expect(listRules(eslint, patterns)).resolves.toHaveLength(47);
await expect(listRules(targets)).resolves.toHaveLength(47);
});

it('should include explicitly set built-in rule', async () => {
await expect(listRules(eslint, patterns)).resolves.toContainEqual({
await expect(listRules(targets)).resolves.toContainEqual({
ruleId: 'no-const-assign',
meta: {
docs: {
Expand All @@ -62,7 +59,7 @@ describe('listRules', () => {
});

it('should include explicitly set plugin rule', async () => {
await expect(listRules(eslint, patterns)).resolves.toContainEqual({
await expect(listRules(targets)).resolves.toContainEqual({
ruleId: 'react/jsx-key',
meta: {
docs: {
Expand Down Expand Up @@ -94,24 +91,21 @@ describe('listRules', () => {
const nxRootDir = join(fixturesDir, 'nx-monorepo');
const eslintrc = join(nxRootDir, 'packages/utils/.eslintrc.json');

const eslint = new ESLint({
useEslintrc: false,
baseConfig: { extends: eslintrc },
});
const patterns = ['packages/utils/**/*.ts', 'packages/utils/**/*.json'];
const targets: ESLintTarget[] = [{ eslintrc, patterns }];

beforeAll(() => {
cwdSpy.mockReturnValue(nxRootDir);
});

it('should list expected number of rules', async () => {
const rules = await listRules(eslint, patterns);
const rules = await listRules(targets);
expect(rules.length).toBeGreaterThanOrEqual(50);
});

it('should include explicitly set plugin rule with custom options', async () => {
// set in root .eslintrc.json
await expect(listRules(eslint, patterns)).resolves.toContainEqual({
await expect(listRules(targets)).resolves.toContainEqual({
ruleId: '@nx/enforce-module-boundaries',
meta: expect.any(Object),
options: [
Expand All @@ -131,7 +125,7 @@ describe('listRules', () => {

it('should include built-in rule set implicitly by extending recommended config', async () => {
// extended via @nx/typescript -> @typescript-eslint/eslint-recommended
await expect(listRules(eslint, patterns)).resolves.toContainEqual({
await expect(listRules(targets)).resolves.toContainEqual({
ruleId: 'no-var',
meta: expect.any(Object),
options: [],
Expand All @@ -140,7 +134,7 @@ describe('listRules', () => {

it('should include plugin rule set implicitly by extending recommended config', async () => {
// extended via @nx/typescript -> @typescript-eslint/recommended
await expect(listRules(eslint, patterns)).resolves.toContainEqual({
await expect(listRules(targets)).resolves.toContainEqual({
ruleId: '@typescript-eslint/no-unused-vars',
meta: expect.any(Object),
options: [],
Expand All @@ -149,7 +143,7 @@ describe('listRules', () => {

it('should not include rule which was turned off in extended config', async () => {
// extended TypeScript config sets "no-unused-semi": "off"
await expect(listRules(eslint, patterns)).resolves.not.toContainEqual(
await expect(listRules(targets)).resolves.not.toContainEqual(
expect.objectContaining({
ruleId: 'no-unused-vars',
} satisfies Partial<RuleData>),
Expand All @@ -158,7 +152,7 @@ describe('listRules', () => {

it('should include rule added to root config by project config', async () => {
// set only in packages/utils/.eslintrc.json
await expect(listRules(eslint, patterns)).resolves.toContainEqual({
await expect(listRules(targets)).resolves.toContainEqual({
ruleId: '@nx/dependency-checks',
meta: expect.any(Object),
options: [],
Expand Down
Loading