Navigation Menu

Skip to content

Commit

Permalink
feat(linter): add support for combo source tags (#13817)
Browse files Browse the repository at this point in the history
Fixes #12871
  • Loading branch information
meeroslav committed Dec 15, 2022
1 parent 3fd1841 commit 5d64b1e
Show file tree
Hide file tree
Showing 4 changed files with 152 additions and 14 deletions.
38 changes: 37 additions & 1 deletion docs/shared/recipes/tag-multiple-dimensions.md
Expand Up @@ -122,8 +122,44 @@ We can now restrict projects within the same group to depend on each other based
}
```

There are no limits to number of tags, but as you add more tags the complexity of your dependency constraints rises exponentially. It's always good to draw a diagram and carefully plan the boundaries.
There are no limits to the number of tags, but as you add more tags the complexity of your dependency constraints rises exponentially. It's always good to draw a diagram and carefully plan the boundaries.

## Matching multiple source tags

Matching just a single source tag is sometimes not enough for solving complex restrictions. To avoid creating ad-hoc tags that are only meant for specific constraints, you can also combine multiple tags with `allSourceTags`. Each tag in the array must be matched for a constraint to be applied:

```jsonc
{
// ... 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": [
{ // this constraint applies to all "admin" projects
"sourceTag": "scope:admin",
"onlyDependOnLibsWithTags": ["scope:shared", "scope:admin"]
},
{
"sourceTag": "type:ui",
"onlyDependOnLibsWithTags": ["type:ui", "type:util"]
},
{ // we don't want our admin ui components to depend on anything except utilities, and we also want to ban router imports
"allSourceTags": ["scope:admin", "type:ui"],
"onlyDependOnLibsWithTags": ["type:util"],
"bannedExternalImports": ["*router*"]
}
]
}
]

// ... more ESLint config here
}

## Further reading

- [Article: Taming Code Organization with Module Boundaries in Nx](https://blog.nrwl.io/mastering-the-project-boundaries-in-nx-f095852f5bf4)
```
Expand Up @@ -749,6 +749,59 @@ Violation detected in:

expect(failures.length).toEqual(0);
});

it('should report errors for combo source tags', () => {
const failures = runRule(
{
depConstraints: [
{
allSourceTags: ['impl', 'domain1'],
onlyDependOnLibsWithTags: ['impl'],
},
{ sourceTag: 'impl', onlyDependOnLibsWithTags: ['api'] },
],
},
// ['impl', 'domain1']
`${process.cwd()}/proj/libs/impl/src/index.ts`,
// ['impl', 'domain1', 'domain2']
`
import '@mycompany/api';
import('@mycompany/api');
`,
graph
);

expect(failures.length).toEqual(2);
expect(failures[0].message).toEqual(
'A project tagged with "impl" and "domain1" can only depend on libs tagged with "impl"'
);
expect(failures[1].message).toEqual(
'A project tagged with "impl" and "domain1" can only depend on libs tagged with "impl"'
);
});

it('should properly map combo source tags', () => {
const failures = runRule(
{
depConstraints: [
{
allSourceTags: ['impl', 'domain1'],
onlyDependOnLibsWithTags: ['api'],
},
],
},
// ['impl', 'domain1']
`${process.cwd()}/proj/libs/impl/src/index.ts`,
// ['impl', 'domain1', 'domain2']
`
import '@mycompany/api';
import('@mycompany/api');
`,
graph
);

expect(failures.length).toEqual(0);
});
});

describe('relative imports', () => {
Expand Down
32 changes: 26 additions & 6 deletions packages/eslint-plugin-nx/src/rules/enforce-module-boundaries.ts
Expand Up @@ -30,6 +30,7 @@ import {
matchImportWithWildcard,
onlyLoadChildren,
stringifyTags,
isComboDepConstraint,
} from '../utils/runtime-lint-utils';
import { AST_NODE_TYPES, TSESTree } from '@typescript-eslint/utils';
import { TargetProjectLocator } from 'nx/src/utils/target-project-locator';
Expand Down Expand Up @@ -91,7 +92,16 @@ export default createESLintRule<Options, MessageIds>({
{
type: 'object',
properties: {
sourceTag: { type: 'string' },
oneOf: [
{ sourceTag: { type: 'string' } },
{
allSourceTags: {
type: 'array',
items: { type: 'string' },
minItems: 2,
},
},
],
onlyDependOnLibsWithTags: [{ type: 'string' }],
bannedExternalImports: [{ type: 'string' }],
notDependOnLibsWithTags: [{ type: 'string' }],
Expand Down Expand Up @@ -389,7 +399,9 @@ export default createESLintRule<Options, MessageIds>({
node,
messageId: 'bannedExternalImportsViolation',
data: {
sourceTag: constraint.sourceTag,
sourceTag: isComboDepConstraint(constraint)
? constraint.allSourceTags.join('" and "')
: constraint.sourceTag,
package: targetProject.data.packageName,
},
});
Expand Down Expand Up @@ -521,7 +533,9 @@ export default createESLintRule<Options, MessageIds>({
node,
messageId: 'onlyTagsConstraintViolation',
data: {
sourceTag: constraint.sourceTag,
sourceTag: isComboDepConstraint(constraint)
? constraint.allSourceTags.join('" and "')
: constraint.sourceTag,
tags: stringifyTags(constraint.onlyDependOnLibsWithTags),
},
});
Expand All @@ -536,7 +550,9 @@ export default createESLintRule<Options, MessageIds>({
node,
messageId: 'emptyOnlyTagsConstraintViolation',
data: {
sourceTag: constraint.sourceTag,
sourceTag: isComboDepConstraint(constraint)
? constraint.allSourceTags.join('" and "')
: constraint.sourceTag,
},
});
return;
Expand All @@ -555,7 +571,9 @@ export default createESLintRule<Options, MessageIds>({
node,
messageId: 'notTagsConstraintViolation',
data: {
sourceTag: constraint.sourceTag,
sourceTag: isComboDepConstraint(constraint)
? constraint.allSourceTags.join('" and "')
: constraint.sourceTag,
tags: stringifyTags(constraint.notDependOnLibsWithTags),
projects: projectPaths
.map(
Expand Down Expand Up @@ -584,7 +602,9 @@ export default createESLintRule<Options, MessageIds>({
node,
messageId: 'bannedExternalImportsViolation',
data: {
sourceTag: constraint.sourceTag,
sourceTag: isComboDepConstraint(constraint)
? constraint.allSourceTags.join('" and "')
: constraint.sourceTag,
childProjectName: violatingSource.name,
package: target.data.packageName,
},
Expand Down
43 changes: 36 additions & 7 deletions packages/eslint-plugin-nx/src/utils/runtime-lint-utils.ts
Expand Up @@ -21,12 +21,21 @@ import {
} from 'nx/src/project-graph/utils/find-project-for-path';

export type Deps = { [projectName: string]: ProjectGraphDependency[] };
export type DepConstraint = {
type SingleSourceTagConstraint = {
sourceTag: string;
onlyDependOnLibsWithTags?: string[];
notDependOnLibsWithTags?: string[];
bannedExternalImports?: string[];
};
type ComboSourceTagConstraint = {
allSourceTags: string[];
onlyDependOnLibsWithTags?: string[];
notDependOnLibsWithTags?: string[];
bannedExternalImports?: string[];
};
export type DepConstraint =
| SingleSourceTagConstraint
| ComboSourceTagConstraint;

export function stringifyTags(tags: string[]): string {
return tags.map((t) => `"${t}"`).join(', ');
Expand All @@ -39,6 +48,12 @@ export function hasNoneOfTheseTags(
return tags.filter((tag) => hasTag(proj, tag)).length === 0;
}

export function isComboDepConstraint(
depConstraint: DepConstraint
): depConstraint is ComboSourceTagConstraint {
return !!(depConstraint as ComboSourceTagConstraint).allSourceTags;
}

/**
* Check if any of the given tags is included in the project
* @param proj ProjectGraphProjectNode
Expand Down Expand Up @@ -151,7 +166,13 @@ export function findConstraintsFor(
depConstraints: DepConstraint[],
sourceProject: ProjectGraphProjectNode
) {
return depConstraints.filter((f) => hasTag(sourceProject, f.sourceTag));
return depConstraints.filter((f) => {
if (isComboDepConstraint(f)) {
return f.allSourceTags.every((tag) => hasTag(sourceProject, tag));
} else {
return hasTag(sourceProject, f.sourceTag);
}
});
}

export function onlyLoadChildren(
Expand Down Expand Up @@ -201,12 +222,20 @@ export function hasBannedImport(
depConstraints: DepConstraint[]
): DepConstraint | undefined {
// return those constraints that match source project and have `bannedExternalImports` defined
depConstraints = depConstraints.filter(
(c) =>
(source.data.tags || []).includes(c.sourceTag) &&
depConstraints = depConstraints.filter((c) => {
let tags = [];
if (isComboDepConstraint(c)) {
tags = c.allSourceTags;
} else {
tags = [c.sourceTag];
}

return (
c.bannedExternalImports &&
c.bannedExternalImports.length
);
c.bannedExternalImports.length &&
tags.every((t) => (source.data.tags || []).includes(t))
);
});
return depConstraints.find((constraint) =>
isConstraintBanningProject(target, constraint)
);
Expand Down

1 comment on commit 5d64b1e

@vercel
Copy link

@vercel vercel bot commented on 5d64b1e Dec 15, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Successfully deployed to the following URLs:

nx-dev – ./

nx-dev-git-master-nrwl.vercel.app
nx-dev-nrwl.vercel.app
nx-five.vercel.app
nx.dev

Please sign in to comment.