Skip to content

Commit 07f52d9

Browse files
leosvelpereznx-cloud[bot]
leosvelperez
and
nx-cloud[bot]
authored
fix(linter): flag banned external imports reached through internal projects (#36654)
## Current Behavior With `checkNestedExternalImports: true`, importing an internal project never reports a violation for that project's transitive external dependencies: - With a `bannedExternalImports` constraint, a child project depending on the banned package produces no violation. Only direct imports of the package are flagged, so the nested check does nothing. - With an `allowedExternalImports` constraint and no ban list, the nested check is skipped entirely. A child project depending on a package outside the allowed list is never reported. ## Expected Behavior Importing an internal project whose transitive external dependencies violate a `bannedExternalImports` or `allowedExternalImports` constraint reports a violation. The error names the violating package and the child project where it was found. Each package is reported once per child project. The allowed list is exclusive on the nested path, as it already is for direct imports. ## Related Issue(s) Fixes #36519 ## Implementation Notes - The subpath-matching guard #17755 added to `isConstraintBanningProject` compares the original import specifier against the external package name. On the nested path the specifier names the imported internal project while the compared package is the transitive external, so the guard never matched. Nested matching now uses each transitive package's own name. The direct import path keeps the specifier-based subpath matching. - Nested matching stays package-granular, same as before #17755, since the project graph records external dependencies at the package level. A `lodash/fp` ban does not flag a child depending on `lodash`. The rule options docs state this. - The nested check ran only when a constraint had a ban list, a gate written before `allowedExternalImports` existed. The gate accepts an allowed list too, including an empty one, which bans every external package. - `findTransitiveExternalDependencies` keeps one dependency edge per project and package. Several edges to the same package (static and dynamic, or several resolved versions) produce a single violation instead of identical repeats. - Rule-level tests cover both directions for banned, allowed-only, mixed and empty-allowed constraints, plus multi-hop chains and the duplicate-edge cases. <!-- polygraph-session-start --> --- <p><a href="https://app.trypolygraph.com/orgs/6a061dcb561c062131116eca/sessions/gh-36519-1f4c8a12">View Polygraph session ↗</a></p> <!-- polygraph-session-end --> --------- Co-authored-by: nx-cloud[bot] <71083854+nx-cloud[bot]@users.noreply.github.com>
1 parent 1fdd76f commit 07f52d9

5 files changed

Lines changed: 320 additions & 69 deletions

File tree

‎astro-docs/src/content/docs/kb/enforce-module-boundaries.mdoc‎

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -73,16 +73,16 @@ export default [
7373

7474
## Options
7575

76-
| Property | Type | Default | Description |
77-
| ---------------------------------- | ------------------------- | ------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
78-
| allow | _Array<string>_ | _[]_ | List of imports that should be allowed without any checks |
79-
| allowCircularSelfDependency | _boolean_ | _false_ | Disable check for self circular dependency when project imports from itself via alias path |
80-
| banTransitiveDependencies | _boolean_ | _false_ | Ban import of dependencies that were not specified in the root or project's `package.json` |
81-
| ignoredCircularDependencies | _Array<[string, string]>_ | _[]_ | List of project pairs that should be skipped from `Circular dependencies` checks, including the self-circular dependency check. E.g. `['feature-project-a', 'myapp']`. Project name can be replaced by catch all `*` for more generic matches. |
82-
| checkDynamicDependenciesExceptions | _Array<string>_ | _[]_ | List of imports that should be skipped for `Imports of lazy-loaded libraries forbidden` checks. E.g. `['@myorg/lazy-project/component/*', '@myorg/other-project']` |
83-
| checkNestedExternalImports | _boolean_ | _false_ | Enable to enforce the check for banned external imports in the nested packages. Check [Dependency constraints](#dependency-constraints) for more information |
84-
| enforceBuildableLibDependency | _boolean_ | _false_ | Enable to restrict the buildable libs from importing non-buildable libraries |
85-
| depConstraints | _Array<object>_ | _[]_ | List of dependency constraints between projects |
76+
| Property | Type | Default | Description |
77+
| ---------------------------------- | ------------------------- | ------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
78+
| allow | _Array<string>_ | _[]_ | List of imports that should be allowed without any checks |
79+
| allowCircularSelfDependency | _boolean_ | _false_ | Disable check for self circular dependency when project imports from itself via alias path |
80+
| banTransitiveDependencies | _boolean_ | _false_ | Ban import of dependencies that were not specified in the root or project's `package.json` |
81+
| ignoredCircularDependencies | _Array<[string, string]>_ | _[]_ | List of project pairs that should be skipped from `Circular dependencies` checks, including the self-circular dependency check. E.g. `['feature-project-a', 'myapp']`. Project name can be replaced by catch all `*` for more generic matches. |
82+
| checkDynamicDependenciesExceptions | _Array<string>_ | _[]_ | List of imports that should be skipped for `Imports of lazy-loaded libraries forbidden` checks. E.g. `['@myorg/lazy-project/component/*', '@myorg/other-project']` |
83+
| checkNestedExternalImports | _boolean_ | _false_ | Enable to also apply the `bannedExternalImports` and `allowedExternalImports` constraints to the external packages that imported projects depend on, transitively. The nested check compares whole package names. In `bannedExternalImports`, a subpath entry such as `lodash/fp` only applies to direct imports. In `allowedExternalImports`, a subpath entry does not allow the package for the nested check, so the rule reports the nested dependency. Check [Dependency constraints](#dependency-constraints) for more information |
84+
| enforceBuildableLibDependency | _boolean_ | _false_ | Enable to restrict the buildable libs from importing non-buildable libraries |
85+
| depConstraints | _Array<object>_ | _[]_ | List of dependency constraints between projects |
8686

8787
### Dependency constraints
8888

‎packages/eslint-plugin/src/rules/enforce-module-boundaries.spec.ts‎

Lines changed: 283 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -664,6 +664,289 @@ describe('Enforce Module Boundaries (eslint)', () => {
664664
expect(failures[1].message).toEqual(message);
665665
});
666666

667+
it('should error when importing a project that transitively depends on a banned external package', () => {
668+
const failures = runRule(
669+
{
670+
depConstraints: [
671+
{ sourceTag: 'api', bannedExternalImports: ['npm-package'] },
672+
],
673+
checkNestedExternalImports: true,
674+
},
675+
`${process.cwd()}/proj/libs/api/src/index.ts`,
676+
`
677+
import '@mycompany/impl';
678+
import('@mycompany/impl');
679+
`,
680+
{
681+
...graph,
682+
dependencies: {
683+
...graph.dependencies,
684+
implName: [
685+
{
686+
source: 'implName',
687+
target: 'npm:npm-package',
688+
type: DependencyType.static,
689+
},
690+
],
691+
},
692+
},
693+
fileMap
694+
);
695+
696+
const message =
697+
'A project tagged with "api" is not allowed to import "@mycompany/impl". Nested import of "npm-package" found at implName';
698+
expect(failures.length).toEqual(2);
699+
expect(failures[0].message).toEqual(message);
700+
expect(failures[1].message).toEqual(message);
701+
});
702+
703+
it('should not error when importing a project whose transitive external dependencies are not banned', () => {
704+
const failures = runRule(
705+
{
706+
depConstraints: [
707+
{ sourceTag: 'api', bannedExternalImports: ['npm-package'] },
708+
],
709+
checkNestedExternalImports: true,
710+
},
711+
`${process.cwd()}/proj/libs/api/src/index.ts`,
712+
`
713+
import '@mycompany/impl';
714+
import('@mycompany/impl');
715+
`,
716+
{
717+
...graph,
718+
dependencies: {
719+
...graph.dependencies,
720+
implName: [
721+
{
722+
source: 'implName',
723+
target: 'npm:npm-awesome-package',
724+
type: DependencyType.static,
725+
},
726+
],
727+
},
728+
},
729+
fileMap
730+
);
731+
732+
expect(failures.length).toEqual(0);
733+
});
734+
735+
describe.each([
736+
{
737+
name: 'a mixed banned and allowed constraint',
738+
constraint: {
739+
sourceTag: 'api',
740+
bannedExternalImports: ['npm-package'],
741+
allowedExternalImports: ['npm-package2'],
742+
},
743+
},
744+
{
745+
name: 'an allowed-only constraint',
746+
constraint: {
747+
sourceTag: 'api',
748+
allowedExternalImports: ['npm-package2'],
749+
},
750+
},
751+
{
752+
name: 'an empty allowed constraint',
753+
constraint: { sourceTag: 'api', allowedExternalImports: [] },
754+
},
755+
])('nested external imports with $name', ({ constraint }) => {
756+
it('should error when importing a project that transitively depends on an external package outside the allowed list', () => {
757+
const failures = runRule(
758+
{ depConstraints: [constraint], checkNestedExternalImports: true },
759+
`${process.cwd()}/proj/libs/api/src/index.ts`,
760+
`
761+
import '@mycompany/impl';
762+
import('@mycompany/impl');
763+
`,
764+
{
765+
...graph,
766+
dependencies: {
767+
...graph.dependencies,
768+
implName: [
769+
{
770+
source: 'implName',
771+
target: 'npm:npm-awesome-package',
772+
type: DependencyType.static,
773+
},
774+
],
775+
},
776+
},
777+
fileMap
778+
);
779+
780+
const message =
781+
'A project tagged with "api" is not allowed to import "@mycompany/impl". Nested import of "npm-awesome-package" found at implName';
782+
expect(failures.length).toEqual(2);
783+
expect(failures[0].message).toEqual(message);
784+
expect(failures[1].message).toEqual(message);
785+
});
786+
});
787+
788+
it('should not error when importing a project whose transitive external dependencies are in the allowed list', () => {
789+
const failures = runRule(
790+
{
791+
depConstraints: [
792+
{ sourceTag: 'api', allowedExternalImports: ['npm-awesome-*'] },
793+
],
794+
checkNestedExternalImports: true,
795+
},
796+
`${process.cwd()}/proj/libs/api/src/index.ts`,
797+
`
798+
import '@mycompany/impl';
799+
import('@mycompany/impl');
800+
`,
801+
{
802+
...graph,
803+
dependencies: {
804+
...graph.dependencies,
805+
implName: [
806+
{
807+
source: 'implName',
808+
target: 'npm:npm-awesome-package',
809+
type: DependencyType.static,
810+
},
811+
],
812+
},
813+
},
814+
fileMap
815+
);
816+
817+
expect(failures.length).toEqual(0);
818+
});
819+
820+
it('should report the project owning the nested import when the banned package is more than one hop away', () => {
821+
const failures = runRule(
822+
{
823+
depConstraints: [
824+
{ sourceTag: 'api', bannedExternalImports: ['npm-package'] },
825+
],
826+
checkNestedExternalImports: true,
827+
},
828+
`${process.cwd()}/proj/libs/api/src/index.ts`,
829+
`
830+
import '@mycompany/impl';
831+
`,
832+
{
833+
...graph,
834+
dependencies: {
835+
...graph.dependencies,
836+
implName: [
837+
{
838+
source: 'implName',
839+
target: 'impl2Name',
840+
type: DependencyType.static,
841+
},
842+
],
843+
impl2Name: [
844+
{
845+
source: 'impl2Name',
846+
target: 'npm:npm-package',
847+
type: DependencyType.static,
848+
},
849+
],
850+
},
851+
},
852+
fileMap
853+
);
854+
855+
expect(failures.length).toEqual(1);
856+
expect(failures[0].message).toEqual(
857+
'A project tagged with "api" is not allowed to import "@mycompany/impl". Nested import of "npm-package" found at impl2Name'
858+
);
859+
});
860+
861+
it('should report a single violation when multiple dependency edges reach the same banned package', () => {
862+
const failures = runRule(
863+
{
864+
depConstraints: [
865+
{ sourceTag: 'api', bannedExternalImports: ['npm-package'] },
866+
],
867+
checkNestedExternalImports: true,
868+
},
869+
`${process.cwd()}/proj/libs/api/src/index.ts`,
870+
`
871+
import '@mycompany/impl';
872+
`,
873+
{
874+
...graph,
875+
dependencies: {
876+
...graph.dependencies,
877+
implName: [
878+
{
879+
source: 'implName',
880+
target: 'npm:npm-package',
881+
type: DependencyType.static,
882+
},
883+
{
884+
source: 'implName',
885+
target: 'npm:npm-package',
886+
type: DependencyType.dynamic,
887+
},
888+
],
889+
},
890+
},
891+
fileMap
892+
);
893+
894+
expect(failures.length).toEqual(1);
895+
expect(failures[0].message).toEqual(
896+
'A project tagged with "api" is not allowed to import "@mycompany/impl". Nested import of "npm-package" found at implName'
897+
);
898+
});
899+
900+
it('should report a single violation when multiple versions of the same banned package are reached', () => {
901+
const failures = runRule(
902+
{
903+
depConstraints: [
904+
{ sourceTag: 'api', bannedExternalImports: ['npm-package'] },
905+
],
906+
checkNestedExternalImports: true,
907+
},
908+
`${process.cwd()}/proj/libs/api/src/index.ts`,
909+
`
910+
import '@mycompany/impl';
911+
`,
912+
{
913+
...graph,
914+
externalNodes: {
915+
...graph.externalNodes,
916+
'npm:npm-package@2.0.0': {
917+
name: 'npm:npm-package@2.0.0',
918+
type: 'npm',
919+
data: {
920+
packageName: 'npm-package',
921+
version: '2.0.0',
922+
},
923+
},
924+
},
925+
dependencies: {
926+
...graph.dependencies,
927+
implName: [
928+
{
929+
source: 'implName',
930+
target: 'npm:npm-package',
931+
type: DependencyType.static,
932+
},
933+
{
934+
source: 'implName',
935+
target: 'npm:npm-package@2.0.0',
936+
type: DependencyType.static,
937+
},
938+
],
939+
},
940+
},
941+
fileMap
942+
);
943+
944+
expect(failures.length).toEqual(1);
945+
expect(failures[0].message).toEqual(
946+
'A project tagged with "api" is not allowed to import "@mycompany/impl". Nested import of "npm-package" found at implName'
947+
);
948+
});
949+
667950
it('should error when importing transitive npm packages', () => {
668951
const failures = runRule(
669952
{

0 commit comments

Comments
 (0)