From ef5813a7b7744978ce12355eee9f8b8b7d01030e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Milo=C5=A1=20Lajtman?= Date: Mon, 19 Jun 2023 17:33:15 +0200 Subject: [PATCH] fix(linter): improve error message for forbidden imports of lazy-loaded libs (#17635) Co-authored-by: Miroslav Jonas --- .../rules/enforce-module-boundaries.spec.ts | 21 +++++++++++--- .../src/rules/enforce-module-boundaries.ts | 14 +++++++++- .../eslint-plugin/src/utils/graph-utils.ts | 28 ++++++++++++++++++- 3 files changed, 57 insertions(+), 6 deletions(-) diff --git a/packages/eslint-plugin/src/rules/enforce-module-boundaries.spec.ts b/packages/eslint-plugin/src/rules/enforce-module-boundaries.spec.ts index 1e3aea0254303..9ed7e57182182 100644 --- a/packages/eslint-plugin/src/rules/enforce-module-boundaries.spec.ts +++ b/packages/eslint-plugin/src/rules/enforce-module-boundaries.spec.ts @@ -1221,20 +1221,33 @@ Violation detected in: target: 'otherName', type: DependencyType.dynamic, }, + { + source: 'mylibName', + target: 'otherName', + type: DependencyType.static, + }, ], }, }, { - mylibName: [createFile(`libs/mylib/src/main.ts`)], + mylibName: [ + createFile(`libs/mylib/src/main.ts`, [ + ['otherName', 'static'], + ['otherName', 'dynamic'], + ]), + ], otherName: [createFile(`libs/other/index.ts`)], } ); if (importKind === 'type') { expect(failures.length).toEqual(0); } else { - expect(failures[0].message).toEqual( - 'Imports of lazy-loaded libraries are forbidden' - ); + expect(failures[0].message).toMatchInlineSnapshot(` + "Static imports of lazy-loaded libraries are forbidden. + + Library "otherName" is lazy-loaded in these files: + - libs/mylib/src/main.ts" + `); } } ); diff --git a/packages/eslint-plugin/src/rules/enforce-module-boundaries.ts b/packages/eslint-plugin/src/rules/enforce-module-boundaries.ts index ce13ded3e700d..925825d07efe0 100644 --- a/packages/eslint-plugin/src/rules/enforce-module-boundaries.ts +++ b/packages/eslint-plugin/src/rules/enforce-module-boundaries.ts @@ -8,6 +8,7 @@ import { import { isRelativePath } from 'nx/src/utils/fileutils'; import { checkCircularPath, + findFilesWithDynamicImports, findFilesInCircularPath, } from '../utils/graph-utils'; import { @@ -125,7 +126,7 @@ export default createESLintRule({ noImportsOfE2e: 'Imports of e2e projects are forbidden', noImportOfNonBuildableLibraries: 'Buildable libraries cannot import or export from non-buildable libraries', - noImportsOfLazyLoadedLibraries: `Imports of lazy-loaded libraries are forbidden`, + noImportsOfLazyLoadedLibraries: `Static imports of lazy-loaded libraries are forbidden.\n\nLibrary "{{targetProjectName}}" is lazy-loaded in these files:\n{{filePaths}}`, projectWithoutTagsCannotHaveDependencies: `A project without tags matching at least one constraint cannot depend on any libraries`, bannedExternalImportsViolation: `A project tagged with "{{sourceTag}}" is not allowed to import the "{{package}}" package`, nestedBannedExternalImportsViolation: `A project tagged with "{{sourceTag}}" is not allowed to import the "{{package}}" package. Nested import found at {{childProjectName}}`, @@ -515,7 +516,18 @@ export default createESLintRule({ [] ) ) { + const filesWithLazyImports = findFilesWithDynamicImports( + projectFileMap, + sourceProject.name, + targetProject.name + ); context.report({ + data: { + filePaths: filesWithLazyImports + .map(({ file }) => `- ${file}`) + .join('\n'), + targetProjectName: targetProject.name, + }, node, messageId: 'noImportsOfLazyLoadedLibraries', }); diff --git a/packages/eslint-plugin/src/utils/graph-utils.ts b/packages/eslint-plugin/src/utils/graph-utils.ts index 8609d6a2b0365..063bd96e7774a 100644 --- a/packages/eslint-plugin/src/utils/graph-utils.ts +++ b/packages/eslint-plugin/src/utils/graph-utils.ts @@ -4,7 +4,11 @@ import type { ProjectGraph, ProjectGraphProjectNode, } from '@nx/devkit'; -import { fileDataDepTarget } from 'nx/src/config/project-graph'; +import { + DependencyType, + fileDataDepTarget, + fileDataDepType, +} from 'nx/src/config/project-graph'; interface Reach { graph: ProjectGraph; @@ -158,3 +162,25 @@ export function findFilesInCircularPath( return filePathChain; } + +export function findFilesWithDynamicImports( + projectFileMap: ProjectFileMap, + sourceProjectName: string, + targetProjectName: string +): FileData[] { + const files: FileData[] = []; + projectFileMap[sourceProjectName].forEach((file) => { + if (!file.deps) return; + if ( + file.deps.some( + (d) => + fileDataDepTarget(d) === targetProjectName && + fileDataDepType(d) === DependencyType.dynamic + ) + ) { + files.push(file); + } + }); + + return files; +}