Skip to content

Commit

Permalink
feat(linter): add target for orphaned parent configs, customize exts,…
Browse files Browse the repository at this point in the history 
… PR feedback
  • Loading branch information
@JamesHenry
JamesHenry committed Mar 8, 2024
1 parent 2a1937c commit 77933fa
Show file tree
Hide file tree
Showing 3 changed files with 174 additions and 43 deletions.
5 changes: 0 additions & 5 deletions packages/devkit/src/utils/update-package-scripts.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import type { PackageJson } from 'nx/src/utils/package-json';
import { basename, dirname } from 'path';
import * as yargs from 'yargs-parser';
import { requireNx } from '../../nx';
import { visitNotIgnoredFiles } from '../generators/visit-not-ignored-files';

const { glob, readJson, readNxJson, workspaceRoot, writeJson } = requireNx();

Expand All @@ -26,10 +25,6 @@ export async function updatePackageScripts(
const nxJson = readNxJson(tree);

const [pattern, createNodes] = createNodesTuple;
const workspaceFiles = [];
visitNotIgnoredFiles(tree, workspaceRoot, (path) => {
workspaceFiles.push(path);
});
const matchingFiles = glob(tree, [pattern]);

for (const file of matchingFiles) {
Expand Down
142 changes: 113 additions & 29 deletions packages/eslint/src/plugins/plugin.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ describe('@nx/eslint/plugin', () => {
});

describe('root eslint config only', () => {
it('should note create any nodes for just a package.json and root level eslint config', async () => {
it('should not create any nodes for just a package.json and root level eslint config', async () => {
applyFilesToVolAndContext(
{
'.eslintrc.json': `{}`,
Expand Down Expand Up @@ -148,15 +148,57 @@ describe('@nx/eslint/plugin', () => {
`);
});

it('should create a node for a nested project which does not have its own eslint config if accompanied by a root level eslint config', async () => {
it('should create a node for a nested project (with a project.json and any lintable file) which does not have its own eslint config if accompanied by a root level eslint config', async () => {
applyFilesToVolAndContext(
{
'.eslintrc.json': `{}`,
'apps/my-app/project.json': `{}`,
// This file is lintable so create the target
'apps/my-app/index.ts': `console.log('hello world')`,
},
context
);
expect(await invokeCreateNodesOnMatchingFiles(context, 'lint'))
.toMatchInlineSnapshot(`
{
"projects": {
"apps/my-app": {
"targets": {
"lint": {
"cache": true,
"command": "eslint .",
"inputs": [
"default",
"^default",
"{workspaceRoot}/.eslintrc.json",
"{workspaceRoot}/tools/eslint-rules/**/*",
{
"externalDependencies": [
"eslint",
],
},
],
"options": {
"cwd": "apps/my-app",
},
},
},
},
},
}
`);
});

it('should create a node for a nested project (with a package.json and any lintable file) which does not have its own eslint config if accompanied by a root level eslint config', async () => {
applyFilesToVolAndContext(
{
'.eslintrc.json': `{}`,
'apps/my-app/package.json': `{}`,
// This file is lintable so create the target
'apps/my-app/index.ts': `console.log('hello world')`,
},
context
);
// NOTE: The command is specifically targeting the src directory in the case of a standalone Nx workspace
expect(await invokeCreateNodesOnMatchingFiles(context, 'lint'))
.toMatchInlineSnapshot(`
{
Expand Down Expand Up @@ -187,6 +229,50 @@ describe('@nx/eslint/plugin', () => {
}
`);
});

it('should not create a node for a nested project (with a package.json and no lintable files) which does not have its own eslint config if accompanied by a root level eslint config', async () => {
applyFilesToVolAndContext(
{
'.eslintrc.json': `{}`,
'apps/my-app/package.json': `{}`,
// These files are not lintable so do not create the target
'apps/my-app/one.png': `...`,
'apps/my-app/two.mov': `...`,
'apps/my-app/three.css': `...`,
'apps/my-app/config-one.yaml': `...`,
'apps/my-app/config-two.yml': `...`,
},
context
);
expect(await invokeCreateNodesOnMatchingFiles(context, 'lint'))
.toMatchInlineSnapshot(`
{
"projects": {},
}
`);
});

it('should not create a node for a nested project (with a project.json and no lintable files) which does not have its own eslint config if accompanied by a root level eslint config', async () => {
applyFilesToVolAndContext(
{
'.eslintrc.json': `{}`,
'apps/my-app/project.json': `{}`,
// These files are not lintable so do not create the target
'apps/my-app/one.png': `...`,
'apps/my-app/two.mov': `...`,
'apps/my-app/three.css': `...`,
'apps/my-app/config-one.yaml': `...`,
'apps/my-app/config-two.yml': `...`,
},
context
);
expect(await invokeCreateNodesOnMatchingFiles(context, 'lint'))
.toMatchInlineSnapshot(`
{
"projects": {},
}
`);
});
});

describe('nested eslint configs only', () => {
Expand All @@ -195,8 +281,10 @@ describe('@nx/eslint/plugin', () => {
{
'apps/my-app/.eslintrc.json': `{}`,
'apps/my-app/project.json': `{}`,
'apps/my-app/index.ts': `console.log('hello world')`,
'libs/my-lib/.eslintrc.json': `{}`,
'libs/my-lib/project.json': `{}`,
'libs/my-lib/index.ts': `console.log('hello world')`,
},
context
);
Expand Down Expand Up @@ -268,11 +356,30 @@ describe('@nx/eslint/plugin', () => {
context
);
// NOTE: The nested projects have the root level config as an input to their lint targets
expect(await invokeCreateNodesOnMatchingFiles(context, 'lint'))
.toMatchInlineSnapshot(`
{
"projects": {},
}
`);
});

it('should create appropriate nodes for a nested project without its own eslint config but with an orphaned eslint config in its parent hierarchy', async () => {
applyFilesToVolAndContext(
{
'.eslintrc.json': '{}',
'apps/.eslintrc.json': '{}',
'apps/myapp/project.json': '{}',
'apps/myapp/index.ts': 'console.log("hello world")',
},
context
);
// NOTE: The nested projects have the root level config as an input to their lint targets
expect(await invokeCreateNodesOnMatchingFiles(context, 'lint'))
.toMatchInlineSnapshot(`
{
"projects": {
"apps/my-app": {
"apps/myapp": {
"targets": {
"lint": {
"cache": true,
Expand All @@ -281,7 +388,7 @@ describe('@nx/eslint/plugin', () => {
"default",
"^default",
"{workspaceRoot}/.eslintrc.json",
"{projectRoot}/.eslintrc.json",
"{workspaceRoot}/apps/.eslintrc.json",
"{workspaceRoot}/tools/eslint-rules/**/*",
{
"externalDependencies": [
Expand All @@ -290,30 +397,7 @@ describe('@nx/eslint/plugin', () => {
},
],
"options": {
"cwd": "apps/my-app",
},
},
},
},
"libs/my-lib": {
"targets": {
"lint": {
"cache": true,
"command": "eslint .",
"inputs": [
"default",
"^default",
"{workspaceRoot}/.eslintrc.json",
"{projectRoot}/.eslintrc.json",
"{workspaceRoot}/tools/eslint-rules/**/*",
{
"externalDependencies": [
"eslint",
],
},
],
"options": {
"cwd": "libs/my-lib",
"cwd": "apps/myapp",
},
},
},
Expand Down
70 changes: 61 additions & 9 deletions packages/eslint/src/plugins/plugin.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,11 @@ import {

export interface EslintPluginOptions {
targetName?: string;
extensions?: string[];
}

const DEFAULT_EXTENSIONS = ['ts', 'tsx', 'js', 'jsx', 'html', 'vue'];

export const createNodes: CreateNodes<EslintPluginOptions> = [
combineGlobPatterns([
...ESLINT_CONFIG_FILENAMES.map((f) => `**/${f}`),
Expand All @@ -28,18 +31,27 @@ export const createNodes: CreateNodes<EslintPluginOptions> = [
(configFilePath, options, context) => {
options = normalizeOptions(options);

// Ensure that configFiles are set, e2e-run fails due to them being undefined in CI
// Ensure that configFiles are set, e2e-run fails due to them being undefined in CI (does not occur locally)
// TODO(JamesHenry): Further troubleshoot this in CI
(context as any).configFiles = context.configFiles ?? [];

const childProjectRoots = globWithWorkspaceContext(context.workspaceRoot, [
'**/project.json',
'**/package.json',
])
.filter(
.map((f) => dirname(f))
.filter((dir) => {
// Ignore root level package.json or project.json
(f) => f !== 'package.json' && f !== 'project.json'
)
.map((f) => dirname(f));
if (dir === '.') {
return false;
}
// Ignore project roots where the project does not contain any lintable files
const lintableFiles = globWithWorkspaceContext(
join(context.workspaceRoot, dir),
[`**/*.{${options.extensions.join(',')}}`]
);
return lintableFiles.length > 0;
});

const configDir = dirname(configFilePath);
if (configDir === '.') {
Expand Down Expand Up @@ -116,25 +128,57 @@ function getProjectsUsingNestedEslintConfig(
): CreateNodesResult['projects'] {
const projects: CreateNodesResult['projects'] = {};
const configDir = dirname(configFilePath);
const configFileInputs = [configFilePath];

if (childProjectRoots.includes(configDir)) {
const configFileInputs = [configFilePath];
// Add the root level config as an input if it exists
const getRootConfig = (): string | undefined => {
const rootFiles = context.configFiles.filter((f) => !f.includes('/'));
const rootEslintConfig = rootFiles.find(
return rootFiles.find(
(f) =>
ESLINT_CONFIG_FILENAMES.some((eslintConfigFile) =>
f.endsWith(eslintConfigFile)
) ||
f === baseEsLintConfigFile ||
f === baseEsLintFlatConfigFile
);
};

// Check if the current config is orphaned (not associated with a project)
const isOrphaned = !childProjectRoots.includes(configDir);

if (isOrphaned) {
const projectRootsUnderConfigDir = childProjectRoots.filter((root) =>
root.startsWith(configDir)
);
// Add the root level config as an input if it exists
const rootEslintConfig = getRootConfig();
if (rootEslintConfig) {
configFileInputs.unshift(rootEslintConfig);
}
// If any of the project roots under the config dir do not have their own eslint config file, add the target
for (const projectRoot of projectRootsUnderConfigDir) {
if (
!ESLINT_CONFIG_FILENAMES.some((eslintConfigFile) =>
context.configFiles.includes(join(projectRoot, eslintConfigFile))
)
) {
projects[projectRoot] = {
targets: buildEslintTargets(configFileInputs, projectRoot, options),
};
}
}
return projects;
}

if (childProjectRoots.includes(configDir)) {
// Add the root level config as an input if it exists
const rootEslintConfig = getRootConfig();
if (rootEslintConfig) {
configFileInputs.unshift(rootEslintConfig);
}
projects[configDir] = {
targets: buildEslintTargets(configFileInputs, configDir, options),
};
return projects;
}

return projects;
Expand Down Expand Up @@ -184,5 +228,13 @@ function buildEslintTargets(
function normalizeOptions(options: EslintPluginOptions): EslintPluginOptions {
options ??= {};
options.targetName ??= 'lint';

// Normalize user input for lintableFiles (strip leading . characters)
if (Array.isArray(options.extensions)) {
options.extensions = options.extensions.map((f) => f.replace(/^\.+/, ''));
} else {
options.extensions = DEFAULT_EXTENSIONS;
}

return options;
}

0 comments on commit 77933fa

Please sign in to comment.