Skip to content

Commit

Permalink
fix(linter): ensure angular entry point checks are correct (#20859)
Browse files Browse the repository at this point in the history
  • Loading branch information
meeroslav authored and jaysoo committed Dec 22, 2023
1 parent 2f5b017 commit 706892b
Show file tree
Hide file tree
Showing 3 changed files with 69 additions and 49 deletions.
4 changes: 2 additions & 2 deletions packages/eslint-plugin/src/rules/enforce-module-boundaries.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ import {
hasBuildExecutor,
hasNoneOfTheseTags,
isAbsoluteImportIntoAnotherProject,
isAngularSecondaryEntrypoint,
belongsToDifferentNgEntryPoint,
isComboDepConstraint,
isDirectDependency,
matchImportWithWildcard,
Expand Down Expand Up @@ -365,7 +365,7 @@ export default ESLintUtils.RuleCreator(() => ``)<Options, MessageIds>({
if (
!allowCircularSelfDependency &&
!isRelativePath(imp) &&
!isAngularSecondaryEntrypoint(
!belongsToDifferentNgEntryPoint(
imp,
sourceFilePath,
sourceProject.data.root
Expand Down
84 changes: 49 additions & 35 deletions packages/eslint-plugin/src/utils/runtime-lint-utils.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import {
hasBannedDependencies,
hasBannedImport,
hasNoneOfTheseTags,
isAngularSecondaryEntrypoint,
belongsToDifferentNgEntryPoint,
isTerminalRun,
} from './runtime-lint-utils';
import { vol } from 'memfs';
Expand Down Expand Up @@ -456,82 +456,96 @@ describe('isAngularSecondaryEntrypoint', () => {
'@project/standard/secondary': [
'libs/standard/secondary/src/index.ts',
],
'@project/standard/tertiary': [
'libs/standard/tertiary/src/public_api.ts',
],
'@project/features': ['libs/features/src/index.ts'],
'@project/features': ['libs/features/index.ts'],
'@project/features/*': ['libs/features/*/random/folder/api.ts'],
'@project/buildable': ['libs/buildable/src/index.ts'],
},
},
};
const fsJson = {
'tsconfig.base.json': JSON.stringify(tsConfig),
'apps/app.ts': '',
'libs/standard/package.json': '{ "version": "0.0.0" }',
'libs/standard/src/index.ts': 'const bla = "foo"',
'libs/standard/secondary/ng-package.json': JSON.stringify({
lib: { entryFile: 'src/index.ts' },
}),
'libs/standard/secondary/src/index.ts': 'const bla = "foo"',
'libs/standard/tertiary/ng-package.json': JSON.stringify({
lib: { entryFile: 'src/public_api.ts' },
}),
'libs/standard/tertiary/src/public_api.ts': 'const bla = "foo"',
'libs/features/package.json': '{ "version": "0.0.0" }',
'libs/features/ng-package.json': JSON.stringify({
lib: { entryFile: 'index.ts' },
}),
'libs/features/index.ts': 'const bla = "foo"',
'libs/features/secondary/ng-package.json': JSON.stringify({
lib: { entryFile: 'random/folder/api.ts' },
}),
'libs/features/secondary/random/folder/api.ts': 'const bla = "foo"',
'libs/buildable/ng-package.json': JSON.stringify({
lib: { entryFile: 'src/index.ts' },
}),
};
vol.fromJSON(fsJson, '/root');
});

it('should return true for secondary entrypoints', () => {
it('should return false if they belong to same entrypoints', () => {
// main
expect(
isAngularSecondaryEntrypoint(
belongsToDifferentNgEntryPoint(
'@project/standard',
'apps/app.ts',
'libs/standard/src/subfolder/index.ts',
'libs/standard'
)
).toBe(false);
expect(
isAngularSecondaryEntrypoint(
belongsToDifferentNgEntryPoint(
'@project/features',
'libs/features/src/subfolder/index.ts',
'libs/features'
)
).toBe(false);
// secondary
expect(
belongsToDifferentNgEntryPoint(
'@project/standard/secondary',
'apps/app.ts',
'libs/standard/secondary/src/subfolder/index.ts',
'libs/standard'
)
).toBe(true);
).toBe(false);
expect(
belongsToDifferentNgEntryPoint(
'@project/features/secondary',
'libs/features/secondary/random/folder/src/index.ts',
'libs/features'
)
).toBe(false);
});

it('should return true if they belong to different entrypoints', () => {
// main
expect(
isAngularSecondaryEntrypoint(
'@project/standard/tertiary',
'apps/app.ts',
belongsToDifferentNgEntryPoint(
'@project/standard',
'libs/standard/secondary/src/subfolder/index.ts',
'libs/standard'
)
).toBe(true);
expect(
isAngularSecondaryEntrypoint(
belongsToDifferentNgEntryPoint(
'@project/features',
'apps/app.ts',
'libs/features/secondary/random/folder/src/index.ts',
'libs/features'
)
).toBe(false);
).toBe(true);
// secondary
expect(
isAngularSecondaryEntrypoint(
'@project/features/secondary',
'apps/app.ts',
'libs/features'
belongsToDifferentNgEntryPoint(
'@project/standard/secondary',
'libs/standard/src/subfolder/index.ts',
'libs/standard'
)
).toBe(true);
expect(
isAngularSecondaryEntrypoint(
'@project/buildable',
'apps/app.ts',
'libs/buildable'
belongsToDifferentNgEntryPoint(
'@project/features/secondary',
'libs/features/src/subfolder/index.ts',
'libs/features'
)
).toBe(false);
).toBe(true);
});
});

Expand Down
30 changes: 18 additions & 12 deletions packages/eslint-plugin/src/utils/runtime-lint-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -463,28 +463,34 @@ export function groupImports(
}

/**
* Checks if import points to a secondary entry point in Angular project
* @param targetProjectLocator
* @param importExpr
* @returns
* Checks if source file belongs to a secondary entry point different than the import one
*/
export function isAngularSecondaryEntrypoint(
export function belongsToDifferentNgEntryPoint(
importExpr: string,
filePath: string,
projectRoot: string
): boolean {
const resolvedModule = resolveModuleByImport(
const resolvedImportFile = resolveModuleByImport(
importExpr,
filePath,
filePath, // not strictly necessary, but speeds up resolution
join(workspaceRoot, getRootTsConfigFileName())
);

return (
!!resolvedModule && fileIsSecondaryEntryPoint(resolvedModule, projectRoot)
if (!resolvedImportFile) {
return false;
}

const importEntryPoint = getAngularEntryPoint(
resolvedImportFile,
projectRoot
);
const srcEntryPoint = getAngularEntryPoint(filePath, projectRoot);

// check if the entry point of import expression is different than the source file's entry point
return importEntryPoint !== srcEntryPoint;
}

function fileIsSecondaryEntryPoint(file: string, projectRoot: string): boolean {
function getAngularEntryPoint(file: string, projectRoot: string): string {
let parent = joinPathFragments(file, '../');
while (parent !== `${projectRoot}/`) {
// we need to find closest existing ng-package.json
Expand All @@ -495,11 +501,11 @@ function fileIsSecondaryEntryPoint(file: string, projectRoot: string): boolean {
if (ngPackageContent) {
// https://github.com/ng-packagr/ng-packagr/blob/23c718d04eea85e015b4c261310b7bd0c39e5311/src/ng-package.schema.json#L54
const entryFile = parseJson(ngPackageContent)?.lib?.entryFile;
return entryFile && file === joinPathFragments(parent, entryFile);
return joinPathFragments(parent, entryFile);
}
parent = joinPathFragments(parent, '../');
}
return false;
return undefined;
}

/**
Expand Down

0 comments on commit 706892b

Please sign in to comment.