From d3e2d616d5da84c1e1ed42a28a535fcb9055dc64 Mon Sep 17 00:00:00 2001 From: Younes Jaaidi Date: Wed, 21 Dec 2022 15:50:39 +0100 Subject: [PATCH] feat(linter): add allowedExternalImports option to boundaries rule (#13891) --- docs/shared/recipes/ban-external-imports.md | 40 +++++++ .../rules/enforce-module-boundaries.spec.ts | 100 ++++++++++++++++++ .../src/rules/enforce-module-boundaries.ts | 1 + .../src/utils/runtime-lint-utils.ts | 26 +++-- 4 files changed, 159 insertions(+), 8 deletions(-) diff --git a/docs/shared/recipes/ban-external-imports.md b/docs/shared/recipes/ban-external-imports.md index c3393107a41dc..7eb69ef1cf3af 100644 --- a/docs/shared/recipes/ban-external-imports.md +++ b/docs/shared/recipes/ban-external-imports.md @@ -59,3 +59,43 @@ Another common example is ensuring that util libraries stay framework-free by ba // ... more ESLint config here } ``` + +## Whitelisting external imports with `allowedExternalImports` + +If you need a more restrictive approach, you can use the `allowedExternalImports` option to ensure that a project only imports from a specific set of packages. +This is useful if you want to enforce separation of concerns _(e.g. keeping your domain logic clean from infrastructure concerns, or ui libraries clean from data access concerns)_ or keep some parts of your codebase framework-free or library-free. + +```jsonc {% fileName=".eslintrc.json" %} +{ + // ... more ESLint config here + + // nx-enforce-module-boundaries should already exist at the top-level of your config + "nx-enforce-module-boundaries": [ + "error", + { + "allow": [], + // update depConstraints based on your tags + "depConstraints": [ + // limiting the dependencies of util libraries to the bare minimum + // projects tagged with "type:util" can only import from "date-fns" + { + "sourceTag": "type:util", + "allowedExternalImports": ["date-fns"] + }, + // ui libraries clean from data access concerns + // projects tagged with "type:ui" can only import pacages matching "@angular/*" except "@angular/common/http" + { + "sourceTag": "type:ui", + "allowedExternalImports": ["@angular/*"], + "bannedExternalImports": ["@angular/common/http"] + }, + // keeping the domain logic clean from infrastructure concerns + // projects tagged with "type:core" can't import any external packages. + { + "sourceTag": "type:core", + "allowedExternalImports": [] + } + ] + } + ] +``` diff --git a/packages/eslint-plugin-nx/src/rules/enforce-module-boundaries.spec.ts b/packages/eslint-plugin-nx/src/rules/enforce-module-boundaries.spec.ts index fde70a6d92626..01611c9e2be46 100644 --- a/packages/eslint-plugin-nx/src/rules/enforce-module-boundaries.spec.ts +++ b/packages/eslint-plugin-nx/src/rules/enforce-module-boundaries.spec.ts @@ -463,6 +463,106 @@ describe('Enforce Module Boundaries (eslint)', () => { expect(failures[1].message).toEqual(message); }); + it('should not error when importing npm packages matching allowed external imports', () => { + const failures = runRule( + { + depConstraints: [ + { sourceTag: 'api', allowedExternalImports: ['npm-package'] }, + ], + }, + `${process.cwd()}/proj/libs/api/src/index.ts`, + ` + import 'npm-package'; + import('npm-package'); + `, + graph + ); + + expect(failures.length).toEqual(0); + }); + + it('should error when importing npm packages not matching allowed external imports', () => { + const failures = runRule( + { + depConstraints: [ + { sourceTag: 'api', allowedExternalImports: ['npm-package'] }, + ], + }, + `${process.cwd()}/proj/libs/api/src/index.ts`, + ` + import 'npm-awesome-package'; + import('npm-awesome-package'); + `, + graph + ); + + const message = + 'A project tagged with "api" is not allowed to import the "npm-awesome-package" package'; + expect(failures.length).toEqual(2); + expect(failures[0].message).toEqual(message); + expect(failures[1].message).toEqual(message); + }); + + it('should not error when importing npm packages matching allowed glob pattern', () => { + const failures = runRule( + { + depConstraints: [ + { sourceTag: 'api', allowedExternalImports: ['npm-awesome-*'] }, + ], + }, + `${process.cwd()}/proj/libs/api/src/index.ts`, + ` + import 'npm-awesome-package'; + import('npm-awesome-package'); + `, + graph + ); + + expect(failures.length).toEqual(0); + }); + + it('should error when importing npm packages not matching allowed glob pattern', () => { + const failures = runRule( + { + depConstraints: [ + { sourceTag: 'api', allowedExternalImports: ['npm-awesome-*'] }, + ], + }, + `${process.cwd()}/proj/libs/api/src/index.ts`, + ` + import 'npm-package'; + import('npm-package'); + `, + graph + ); + + const message = + 'A project tagged with "api" is not allowed to import the "npm-package" package'; + expect(failures.length).toEqual(2); + expect(failures[0].message).toEqual(message); + expect(failures[1].message).toEqual(message); + }); + + it('should error when importing any npm package if none is allowed', () => { + const failures = runRule( + { + depConstraints: [{ sourceTag: 'api', allowedExternalImports: [] }], + }, + `${process.cwd()}/proj/libs/api/src/index.ts`, + ` + import 'npm-package'; + import('npm-package'); + `, + graph + ); + + const message = + 'A project tagged with "api" is not allowed to import the "npm-package" package'; + expect(failures.length).toEqual(2); + expect(failures[0].message).toEqual(message); + expect(failures[1].message).toEqual(message); + }); + it('should error when importing transitive npm packages', () => { const failures = runRule( { diff --git a/packages/eslint-plugin-nx/src/rules/enforce-module-boundaries.ts b/packages/eslint-plugin-nx/src/rules/enforce-module-boundaries.ts index 81273dc9b9c3e..4d2a09f1ece38 100644 --- a/packages/eslint-plugin-nx/src/rules/enforce-module-boundaries.ts +++ b/packages/eslint-plugin-nx/src/rules/enforce-module-boundaries.ts @@ -103,6 +103,7 @@ export default createESLintRule({ }, ], onlyDependOnLibsWithTags: [{ type: 'string' }], + allowedExternalImports: [{ type: 'string' }], bannedExternalImports: [{ type: 'string' }], notDependOnLibsWithTags: [{ type: 'string' }], }, diff --git a/packages/eslint-plugin-nx/src/utils/runtime-lint-utils.ts b/packages/eslint-plugin-nx/src/utils/runtime-lint-utils.ts index 02ca8a41137e2..d14956e6df9c7 100644 --- a/packages/eslint-plugin-nx/src/utils/runtime-lint-utils.ts +++ b/packages/eslint-plugin-nx/src/utils/runtime-lint-utils.ts @@ -25,12 +25,14 @@ type SingleSourceTagConstraint = { sourceTag: string; onlyDependOnLibsWithTags?: string[]; notDependOnLibsWithTags?: string[]; + allowedExternalImports?: string[]; bannedExternalImports?: string[]; }; type ComboSourceTagConstraint = { allSourceTags: string[]; onlyDependOnLibsWithTags?: string[]; notDependOnLibsWithTags?: string[]; + allowedExternalImports?: string[]; bannedExternalImports?: string[]; }; export type DepConstraint = @@ -209,10 +211,22 @@ function isConstraintBanningProject( externalProject: ProjectGraphExternalNode, constraint: DepConstraint ): boolean { - return constraint.bannedExternalImports.some((importDefinition) => - parseImportWildcards(importDefinition).test( - externalProject.data.packageName + const { allowedExternalImports, bannedExternalImports } = constraint; + const { packageName } = externalProject.data; + + /* Check if import is banned... */ + if ( + bannedExternalImports?.some((importDefinition) => + parseImportWildcards(importDefinition).test(packageName) ) + ) { + return true; + } + + /* ... then check if there is a whitelist and if there is a match in the whitelist. */ + return allowedExternalImports?.every( + (importDefinition) => + !parseImportWildcards(importDefinition).test(packageName) ); } @@ -230,11 +244,7 @@ export function hasBannedImport( tags = [c.sourceTag]; } - return ( - c.bannedExternalImports && - c.bannedExternalImports.length && - tags.every((t) => (source.data.tags || []).includes(t)) - ); + return tags.every((t) => (source.data.tags || []).includes(t)); }); return depConstraints.find((constraint) => isConstraintBanningProject(target, constraint)