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 721be4b1fd..d7310d1efe 100644 --- a/packages/eslint-plugin/src/rules/enforce-module-boundaries.spec.ts +++ b/packages/eslint-plugin/src/rules/enforce-module-boundaries.spec.ts @@ -47,6 +47,7 @@ const tsconfig = { '@mycompany/other': ['libs/other/src/index.ts'], '@mycompany/other/a/b': ['libs/other/src/a/b.ts'], '@mycompany/other/a': ['libs/other/src/a/index.ts'], + '@mycompany/other/secondary': ['libs/other/src/secondary.ts'], '@mycompany/another/a/b': ['libs/another/a/b.ts'], '@mycompany/myapp': ['apps/myapp/src/index.ts'], '@mycompany/myapp-e2e': ['apps/myapp-e2e/src/index.ts'], @@ -92,6 +93,13 @@ const fileSys = { './libs/other/src/index.ts': '', './libs/other/src/a/b.ts': '', './libs/other/src/a/index.ts': '', + './libs/other/src/secondary.ts': '', + './libs/other/package.json': JSON.stringify({ + exports: { + './secondary': './src/secondary.ts', + '.': './src/index.ts', + }, + }), './libs/another/a/b.ts': '', './apps/myapp/src/index.ts': '', './libs/mylib/src/index.ts': '', @@ -439,10 +447,6 @@ describe('Enforce Module Boundaries (eslint)', () => { ], }; - beforeEach(() => { - vol.fromJSON(fileSys, '/root'); - }); - it('should error when the target library does not have the right tag', () => { const failures = runRule( depConstraints, @@ -1391,6 +1395,126 @@ Violation detected in: } ); + it('should not error when statically importing dynamic dependencies if it belongs to different entry point', () => { + const failures = runRule( + {}, + `${process.cwd()}/proj/libs/mylib/src/main.ts`, + 'import { someValue } from "@mycompany/other/secondary";', + { + nodes: { + mylibName: { + name: 'mylibName', + type: 'lib', + data: { + root: 'libs/mylib', + tags: [], + implicitDependencies: [], + targets: {}, + }, + }, + otherName: { + name: 'otherName', + type: 'lib', + data: { + root: 'libs/other', + tags: [], + implicitDependencies: [], + targets: {}, + }, + }, + }, + dependencies: { + mylibName: [ + { + source: 'mylibName', + target: 'otherName', + type: DependencyType.dynamic, + }, + { + source: 'mylibName', + target: 'otherName', + type: DependencyType.static, + }, + ], + }, + }, + { + mylibName: [ + createFile(`libs/mylib/src/main.ts`, [ + ['otherName', DependencyType.static], + ['otherName', DependencyType.dynamic], + ]), + ], + otherName: [ + createFile(`libs/other/src/index.ts`), + createFile(`libs/other/src/secondary.ts`), + createFile(`libs/other/package.json`), + ], + } + ); + expect(failures.length).toEqual(0); + }); + + it('should error when statically importing dynamic dependencies if it belongs to root entry point', () => { + const failures = runRule( + {}, + `${process.cwd()}/proj/libs/mylib/src/main.ts`, + 'import { someValue } from "@mycompany/other";', + { + nodes: { + mylibName: { + name: 'mylibName', + type: 'lib', + data: { + root: 'libs/mylib', + tags: [], + implicitDependencies: [], + targets: {}, + }, + }, + otherName: { + name: 'otherName', + type: 'lib', + data: { + root: 'libs/other', + tags: [], + implicitDependencies: [], + targets: {}, + }, + }, + }, + dependencies: { + mylibName: [ + { + source: 'mylibName', + target: 'otherName', + type: DependencyType.dynamic, + }, + { + source: 'mylibName', + target: 'otherName', + type: DependencyType.static, + }, + ], + }, + }, + { + mylibName: [ + createFile(`libs/mylib/src/main.ts`, [ + ['otherName', DependencyType.static], + ['otherName', DependencyType.dynamic], + ]), + ], + otherName: [ + createFile(`libs/other/src/index.ts`), + createFile(`libs/other/src/secondary.ts`), + createFile(`libs/other/package.json`), + ], + } + ); + expect(failures.length).toEqual(1); + }); + it('should error on importing an app', () => { const failures = runRule( {}, diff --git a/packages/eslint-plugin/src/rules/enforce-module-boundaries.ts b/packages/eslint-plugin/src/rules/enforce-module-boundaries.ts index 9ac1269f1f..bdd75faf60 100644 --- a/packages/eslint-plugin/src/rules/enforce-module-boundaries.ts +++ b/packages/eslint-plugin/src/rules/enforce-module-boundaries.ts @@ -29,7 +29,7 @@ import { import { readProjectGraph } from '../utils/project-graph-utils'; import { appIsMFERemote, - belongsToDifferentNgEntryPoint, + belongsToDifferentEntryPoint, DepConstraint, findConstraintsFor, findDependenciesWithTags, @@ -412,7 +412,7 @@ export default ESLintUtils.RuleCreator( if ( !allowCircularSelfDependency && !isRelativePath(imp) && - !belongsToDifferentNgEntryPoint( + !belongsToDifferentEntryPoint( imp, sourceFilePath, sourceProject.data.root @@ -623,7 +623,9 @@ export default ESLintUtils.RuleCreator( node, projectGraph, sourceProject.name, - targetProject.name + targetProject.name, + imp, + sourceFilePath ) ) { const filesWithLazyImports = findFilesWithDynamicImports( diff --git a/packages/eslint-plugin/src/utils/runtime-lint-utils.spec.ts b/packages/eslint-plugin/src/utils/runtime-lint-utils.spec.ts index ec63d5959c..97dfd5f759 100644 --- a/packages/eslint-plugin/src/utils/runtime-lint-utils.spec.ts +++ b/packages/eslint-plugin/src/utils/runtime-lint-utils.spec.ts @@ -14,8 +14,9 @@ import { hasBannedDependencies, hasBannedImport, hasNoneOfTheseTags, - belongsToDifferentNgEntryPoint, + belongsToDifferentEntryPoint, isTerminalRun, + parseExports, } from './runtime-lint-utils'; import { vol } from 'memfs'; @@ -486,14 +487,14 @@ describe('isAngularSecondaryEntrypoint', () => { it('should return false if they belong to same entrypoints', () => { // main expect( - belongsToDifferentNgEntryPoint( + belongsToDifferentEntryPoint( '@project/standard', 'libs/standard/src/subfolder/index.ts', 'libs/standard' ) ).toBe(false); expect( - belongsToDifferentNgEntryPoint( + belongsToDifferentEntryPoint( '@project/features', 'libs/features/src/subfolder/index.ts', 'libs/features' @@ -501,14 +502,14 @@ describe('isAngularSecondaryEntrypoint', () => { ).toBe(false); // secondary expect( - belongsToDifferentNgEntryPoint( + belongsToDifferentEntryPoint( '@project/standard/secondary', 'libs/standard/secondary/src/subfolder/index.ts', 'libs/standard' ) ).toBe(false); expect( - belongsToDifferentNgEntryPoint( + belongsToDifferentEntryPoint( '@project/features/secondary', 'libs/features/secondary/random/folder/src/index.ts', 'libs/features' @@ -519,14 +520,14 @@ describe('isAngularSecondaryEntrypoint', () => { it('should return true if they belong to different entrypoints', () => { // main expect( - belongsToDifferentNgEntryPoint( + belongsToDifferentEntryPoint( '@project/standard', 'libs/standard/secondary/src/subfolder/index.ts', 'libs/standard' ) ).toBe(true); expect( - belongsToDifferentNgEntryPoint( + belongsToDifferentEntryPoint( '@project/features', 'libs/features/secondary/random/folder/src/index.ts', 'libs/features' @@ -534,14 +535,14 @@ describe('isAngularSecondaryEntrypoint', () => { ).toBe(true); // secondary expect( - belongsToDifferentNgEntryPoint( + belongsToDifferentEntryPoint( '@project/standard/secondary', 'libs/standard/src/subfolder/index.ts', 'libs/standard' ) ).toBe(true); expect( - belongsToDifferentNgEntryPoint( + belongsToDifferentEntryPoint( '@project/features/secondary', 'libs/features/src/subfolder/index.ts', 'libs/features' @@ -657,3 +658,88 @@ describe('appIsMFERemote', () => { expect(appIsMFERemote(targetNone)).toBe(false); }); }); + +describe('parseExports', () => { + it('should return empty array if exports is a string', () => { + const result = []; + parseExports('index.js', '/root', result); + expect(result).toEqual([]); + }); + it('should return empty array if only default conditional exports', () => { + const result = []; + parseExports({ default: 'index.js', import: 'index.mjs' }, '/root', result); + expect(result).toEqual([]); + }); + it('should return empty array if only default require exports', () => { + const result = []; + parseExports({ require: 'index.cjs' }, '/root', result); + expect(result).toEqual([]); + }); + it('should return empty array if only default import exports', () => { + const result = []; + parseExports({ import: 'index.mjs' }, '/root', result); + expect(result).toEqual([]); + }); + it('should return empty array if only default import exports', () => { + const result = []; + parseExports({ '.': 'index.js' }, '/root', result); + expect(result).toEqual([]); + }); + it('should return secondary entry point if exists', () => { + const result = []; + parseExports( + { '.': './src/index.js', './secondary': './src/secondary.js' }, + '/root', + result + ); + expect(result).toEqual([ + { file: '/root/src/secondary.js', path: '/root/secondary' }, + ]); + }); + it('should return nested secondary entry point with default export', () => { + const result = []; + parseExports( + { '.': './src/index.js', './secondary': './src/secondary.js' }, + '/root', + result + ); + expect(result).toEqual([ + { file: '/root/src/secondary.js', path: '/root/secondary' }, + ]); + }); + it('should return nested conditionalsecondary entry point with default export', () => { + const result = []; + parseExports( + { + '.': './src/index.js', + './secondary': { + default: './src/secondary.js', + import: './src/secondary.mjs', + require: './src/secondary.cjs', + }, + }, + '/root', + result + ); + expect(result).toEqual([ + { file: '/root/src/secondary.js', path: '/root/secondary' }, + ]); + }); + it('should ignore root and null exports', () => { + const result = []; + parseExports( + { + '.': './src/index.js', + './secondary': './src/secondary.js', + './tertiary': './src/tertiary.js', + './tertiary/private': null, + }, + '/root', + result + ); + expect(result).toEqual([ + { file: '/root/src/secondary.js', path: '/root/secondary' }, + { file: '/root/src/tertiary.js', path: '/root/tertiary' }, + ]); + }); +}); diff --git a/packages/eslint-plugin/src/utils/runtime-lint-utils.ts b/packages/eslint-plugin/src/utils/runtime-lint-utils.ts index 96f5355a39..ce960836ba 100644 --- a/packages/eslint-plugin/src/utils/runtime-lint-utils.ts +++ b/packages/eslint-plugin/src/utils/runtime-lint-utils.ts @@ -210,7 +210,9 @@ export function hasStaticImportOfDynamicResource( | TSESTree.ExportNamedDeclaration, graph: ProjectGraph, sourceProjectName: string, - targetProjectName: string + targetProjectName: string, + importExpr: string, + filePath: string ): boolean { if ( node.type !== AST_NODE_TYPES.ImportDeclaration || @@ -218,10 +220,17 @@ export function hasStaticImportOfDynamicResource( ) { return false; } - return onlyLoadChildren(graph, sourceProjectName, targetProjectName, []); + return ( + hasDynamicImport(graph, sourceProjectName, targetProjectName, []) && + !getSecondaryEntryPointPath( + importExpr, + filePath, + graph.nodes[targetProjectName].data.root + ) + ); } -function onlyLoadChildren( +function hasDynamicImport( graph: ProjectGraph, sourceProjectName: string, targetProjectName: string, @@ -238,7 +247,7 @@ function onlyLoadChildren( if (d.target === targetProjectName) { return true; } - return onlyLoadChildren(graph, d.target, targetProjectName, [ + return hasDynamicImport(graph, d.target, targetProjectName, [ ...visited, sourceProjectName, ]); @@ -490,35 +499,57 @@ export function groupImports( /** * Checks if source file belongs to a secondary entry point different than the import one */ -export function belongsToDifferentNgEntryPoint( +export function belongsToDifferentEntryPoint( importExpr: string, filePath: string, projectRoot: string ): boolean { - const resolvedImportFile = resolveModuleByImport( + const importEntryPoint = getSecondaryEntryPointPath( importExpr, - filePath, // not strictly necessary, but speeds up resolution - path.join(workspaceRoot, getRootTsConfigFileName()) - ); - - if (!resolvedImportFile) { - return false; - } - - const importEntryPoint = getAngularEntryPoint( - resolvedImportFile, + filePath, projectRoot ); - const srcEntryPoint = getAngularEntryPoint(filePath, projectRoot); + const srcEntryPoint = getEntryPoint(filePath, projectRoot); // check if the entry point of import expression is different than the source file's entry point return importEntryPoint !== srcEntryPoint; } -function getAngularEntryPoint(file: string, projectRoot: string): string { +export function getSecondaryEntryPointPath( + importExpr: string, + filePath: string, + projectRoot: string +): string | undefined { + const resolvedImportFile = resolveModuleByImport( + importExpr, + filePath, // not strictly necessary, but speeds up resolution + path.join(workspaceRoot, getRootTsConfigFileName()) + ); + if (!resolvedImportFile) { + return undefined; + } + const entryPoint = getEntryPoint(resolvedImportFile, projectRoot); + return entryPoint; +} + +function getEntryPoint(file: string, projectRoot: string): string { + const packageEntryPoints = getPackageEntryPoints(projectRoot); + const fileEntryPoint = packageEntryPoints.find( + (entry) => entry.file === file + ); + if (fileEntryPoint) { + return fileEntryPoint.file; + } + let parent = joinPathFragments(file, '../'); while (parent !== `${projectRoot}/`) { - // we need to find closest existing ng-package.json + const entryPoint = packageEntryPoints.find( + (entry) => entry.path === parent + ); + if (entryPoint) { + return entryPoint.file; + } + // for Angular we need to find closest existing ng-package.json // in order to determine if the file matches the secondary entry point const ngPackageContent = readFileIfExisting( path.join(workspaceRoot, parent, 'ng-package.json') @@ -533,6 +564,62 @@ function getAngularEntryPoint(file: string, projectRoot: string): string { return undefined; } +function getPackageEntryPoints( + projectRoot: string +): Array<{ path: string; file: string }> { + const packageContent = readFileIfExisting( + path.join(workspaceRoot, projectRoot, 'package.json') + ); + if (!packageContent) { + return []; + } + const exports = parseJson(packageContent).exports; + if (!exports) { + return []; + } + const entryPaths: Array<{ path: string; file: string }> = []; + parseExports(exports, projectRoot, entryPaths); + return entryPaths; +} + +export function parseExports( + exports: string | null | Record, + projectRoot: string, + entryPaths: Array<{ path: string; file: string }>, + basePath: string = '.' +): Array<{ path: string; file: string }> { + if (exports === null) { + return; + } + if (typeof exports === 'string') { + if (basePath === '.') { + return; + } else { + entryPaths.push({ + path: joinPathFragments(projectRoot, basePath), + file: joinPathFragments(projectRoot, exports), + }); + return; + } + } + + // parse conditional exports + if (exports.import || exports.require || exports.default || exports.node) { + parseExports( + exports.default || exports.import || exports.require || exports.node, + projectRoot, + entryPaths, + basePath + ); + return; + } + + // parse general nested exports + for (const [key, value] of Object.entries(exports)) { + parseExports(value, projectRoot, entryPaths, key); + } +} + /** * Returns true if the given project contains MFE config with "exposes:" section */