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

feat(linter): add internal rule to prevent deep import from plugins/js #16253

Closed
wants to merge 4 commits into from
Closed
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
6 changes: 6 additions & 0 deletions .eslintrc.json
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,12 @@
"rules": {
"@nrwl/nx/workspace/valid-schema-description": "error"
}
},
{
"files": ["*.ts", "*.tsx", "*.js", "*.jsx"],
"rules": {
"@nrwl/nx/workspace/restrict-nx-imports": "error"
}
}
]
}
2 changes: 1 addition & 1 deletion packages/devkit/src/utils/module-federation/share.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import {
collectPackageSecondaryEntryPoints,
collectWorkspaceLibrarySecondaryEntryPoints,
} from './secondary-entry-points';
// eslint-disable-next-line @typescript-eslint/no-restricted-imports
// eslint-disable-next-line @typescript-eslint/no-restricted-imports, @nrwl/nx/workspace/restrict-nx-imports
import { getRootTsConfigPath } from 'nx/src/plugins/js/utils/typescript';
import type { ProjectGraph } from 'nx/src/config/project-graph';
import { requireNx } from '../../../nx';
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { existsSync } from 'fs';
import { ParsedCommandLine } from 'typescript';
// eslint-disable-next-line @typescript-eslint/no-restricted-imports
// eslint-disable-next-line @typescript-eslint/no-restricted-imports, @nrwl/nx/workspace/restrict-nx-imports
import { getRootTsConfigPath } from 'nx/src/plugins/js/utils/typescript';
import { dirname } from 'path';

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import { DependencyType } from '@nx/devkit';
import * as parser from '@typescript-eslint/parser';
import { TSESLint } from '@typescript-eslint/utils';
import { vol } from 'memfs';
// eslint-disable-next-line @nrwl/nx/workspace/restrict-nx-imports
import { TargetProjectLocator } from 'nx/src/plugins/js/project-graph/build-dependencies/target-project-locator';
import enforceModuleBoundaries, {
RULE_NAME as enforceModuleBoundariesRuleName,
Expand Down
2 changes: 1 addition & 1 deletion packages/eslint-plugin-nx/src/utils/ast-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import {
ProjectGraphProjectNode,
readJsonFile,
} from '@nx/devkit';
import { findNodes } from 'nx/src/plugins/js/utils/typescript';
import { findNodes } from '@nx/js';
import { existsSync, readFileSync } from 'fs';
import { dirname } from 'path';
import ts = require('typescript');
Expand Down
7 changes: 2 additions & 5 deletions packages/js/src/utils/package-json/update-package-json.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,5 @@
import {
createLockFile,
getLockFileName,
} from 'nx/src/plugins/js/lock-file/lock-file';
import { createPackageJson } from 'nx/src/plugins/js/package-json/create-package-json';
import { createLockFile, getLockFileName } from 'nx/src/plugins/js';
import { createPackageJson } from 'nx/src/plugins/js';
import {
ExecutorContext,
getOutputsForTargetAndConfiguration,
Expand Down
4 changes: 2 additions & 2 deletions packages/nx/src/generators/utils/project-configuration.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { basename, dirname, join, relative } from 'path';
import { basename, join, relative } from 'path';
import {
ProjectConfiguration,
ProjectsConfigurations,
Expand All @@ -18,7 +18,7 @@ import { readJson, writeJson } from './json';
import { PackageJson } from '../../utils/package-json';
import { readNxJson } from './nx-json';
import { output } from '../../utils/output';
import { getNxRequirePaths } from 'nx/src/utils/installation-directory';
import { getNxRequirePaths } from '../../utils/installation-directory';

export { readNxJson, updateNxJson } from './nx-json';
export {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import {
import { dirname } from 'path';
import { readJson, writeJson } from '../../generators/utils/json';
import { formatChangedFilesWithPrettierIfAvailable } from '../../generators/internal-utils/format-changed-files-with-prettier-if-available';
import { getNxRequirePaths } from 'nx/src/utils/installation-directory';
import { getNxRequirePaths } from '../../utils/installation-directory';

export default async function (tree: Tree) {
const nxJson = readNxJson(tree);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,13 +1,11 @@
import {
addProjectConfiguration,
getProjects,
readNxJson,
readProjectConfiguration,
} from '../../generators/utils/project-configuration';
import { Tree } from '../../generators/tree';

import update from './update-depends-on-to-tokens';
import { updateJson, writeJson } from 'nx/src/devkit-exports';
import { updateJson, writeJson } from '../../generators/utils/json';
import { createTreeWithEmptyWorkspace } from '../../generators/testing-utils/create-tree-with-empty-workspace';

describe('update-depends-on-to-tokens', () => {
Expand Down
3 changes: 3 additions & 0 deletions packages/nx/src/plugins/js/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,9 @@ import { workspaceRoot } from '../../utils/workspace-root';
import { ensureDirSync } from 'fs-extra';
import { removeNpmNodes } from './lock-file/remove-npm-nodes';

export { getLockFileName, createLockFile } from './lock-file/lock-file';
export { createPackageJson } from './package-json/create-package-json';

export const processProjectGraph: ProjectGraphProcessor = async (
graph,
context
Expand Down
2 changes: 1 addition & 1 deletion packages/nx/src/plugins/js/lock-file/pnpm-parser.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import { parsePnpmLockfile, stringifyPnpmLockfile } from './pnpm-parser';
import { ProjectGraph } from '../../../config/project-graph';
import { vol } from 'memfs';
import { pruneProjectGraph } from './project-graph-pruning';
import { ProjectGraphBuilder } from 'nx/src/project-graph/project-graph-builder';
import { ProjectGraphBuilder } from '../../../project-graph/project-graph-builder';

jest.mock('fs', () => require('memfs').fs);

Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { ProjectGraphProjectNode } from 'nx/src/config/project-graph';
import { ProjectGraphProjectNode } from '../../config/project-graph';
import { normalizeImplicitDependencies } from './workspace-projects';

describe('workspace-projects', () => {
Expand Down
9 changes: 8 additions & 1 deletion tools/eslint-rules/index.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
import {
RULE_NAME as restrictNxImportsName,
rule as restrictNxImports,
} from './rules/restrict-nx-imports';
import {
RULE_NAME as validSchemaDescriptionName,
rule as validSchemaDescription,
Expand Down Expand Up @@ -27,5 +31,8 @@ module.exports = {
* [myCustomRuleName]: myCustomRule
* }
*/
rules: { [validSchemaDescriptionName]: validSchemaDescription },
rules: {
[validSchemaDescriptionName]: validSchemaDescription,
[restrictNxImportsName]: restrictNxImports,
},
};
40 changes: 40 additions & 0 deletions tools/eslint-rules/rules/restrict-nx-imports.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
import { TSESLint } from '@typescript-eslint/utils';
import { rule, RULE_NAME } from './restrict-nx-imports';

const ruleTester = new TSESLint.RuleTester({
parser: require.resolve('@typescript-eslint/parser'),
});

jest.mock('@nrwl/devkit', () => ({
normalizePath: (path: string) => path,
workspaceRoot: '/root',
}));

ruleTester.run(RULE_NAME, rule, {
valid: [
{
code: `import { createLockFile } from 'nx/src/plugins/js';`,
filename: '/root/packages/devkit/src/path/to.ts',
},
{
code: `import { createLockFile } from '../plugins/js';`,
filename: '/root/packages/nx/src/path/to.ts',
},
{
code: `import { updateJson } from '../../generators/utils/json';`,
filename: '/root/packages/nx/src/path/to.ts',
},
],
invalid: [
{
errors: [{ messageId: 'noJsImport' }],
code: `import { createLockFile } from 'nx/src/plugins/js/lock-file/lock-file';`,
filename: '/root/packages/storybook/src/path/to.ts',
},
{
errors: [{ messageId: 'noCircularNx' }],
code: `import { updateJson } from 'nx/src/devkit-exports';`,
filename: '/root/packages/nx/src/path/to.ts',
},
],
});
107 changes: 107 additions & 0 deletions tools/eslint-rules/rules/restrict-nx-imports.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,107 @@
/**
* This file sets you up with structure needed for an ESLint rule.
*
* It leverages utilities from @typescript-eslint to allow TypeScript to
* provide autocompletions etc for the configuration.
*
* Your rule's custom logic will live within the create() method below
* and you can learn more about writing ESLint rules on the official guide:
*
* https://eslint.org/docs/developer-guide/working-with-rules
*
* You can also view many examples of existing rules here:
*
* https://github.com/typescript-eslint/typescript-eslint/tree/master/packages/eslint-plugin/src/rules
*/

import { normalizePath, workspaceRoot } from '@nrwl/devkit';
import {
ESLintUtils,
AST_NODE_TYPES,
TSESTree,
} from '@typescript-eslint/utils';

// NOTE: The rule will be available in ESLint configs as "@nrwl/nx/workspace/restrict-nx-imports"
export const RULE_NAME = 'restrict-nx-imports';

export const rule = ESLintUtils.RuleCreator(() => __filename)({
name: RULE_NAME,
meta: {
type: 'problem',
docs: {
description: `Ensure that there are no deep imports from nx/src/plugins/js`,
recommended: 'error',
},
schema: [],
messages: {
noCircularNx:
'Functions within "nx" should be imported with relative path. Alias import found: {{imp}}',
noJsImport:
'Functions from "nx/src/plugins/js" should be imported from "@nrwl/js". Direct import found: {{imp}}',

// TODO add deep import check from non js to nx/src/plugins/js
},
},
defaultOptions: [],
create(context) {
function run(
node:
| TSESTree.ImportDeclaration
| TSESTree.ImportExpression
| TSESTree.ExportAllDeclaration
| TSESTree.ExportNamedDeclaration
) {
// Ignoring ExportNamedDeclarations like:
// export class Foo {}
if (!node.source) {
return;
}

// accept only literals because template literals have no value
if (node.source.type !== AST_NODE_TYPES.Literal) {
return;
}
const imp = node.source.value as string;
const fileName = normalizePath(context.getFilename()).slice(
workspaceRoot.length + 1
);

if (
imp.includes('nx/src/plugins/js/') &&
!fileName.startsWith('packages/js/')
) {
context.report({
node,
messageId: 'noJsImport',
data: {
imp,
},
});
}
if (imp.includes('nx/src') && fileName.startsWith('packages/nx/')) {
context.report({
node,
messageId: 'noCircularNx',
data: {
imp,
},
});
}
}

return {
ImportDeclaration(node: TSESTree.ImportDeclaration) {
run(node);
},
ImportExpression(node: TSESTree.ImportExpression) {
run(node);
},
ExportAllDeclaration(node: TSESTree.ExportAllDeclaration) {
run(node);
},
ExportNamedDeclaration(node: TSESTree.ExportNamedDeclaration) {
run(node);
},
};
},
});