feat(linter): add support for package based secondary entry points (#30809)
This PR adds support for package.json based secondary entry points and implements fix for situation when package imports base entry point as dynamic dependency and secondary entry point as static dependency. ## Current Behavior When the package is imported from itself, check for a secondary entry point checks only Angular-style secondary entry points. When package is importing from the same library as dynamic import from root and static import from secondary entry point we still get linter errror. ## Expected Behavior Check for secondary entry points should also support standard package.json-based entry points. Importing from the same library as dynamic import from root and static import from secondary entry point should be allowed. ## Related Issue(s) <!-- Please link the issue being fixed so it gets closed when this is merged. --> Fixes #18552
This commit is contained in:
Miroslav Jonaš
GitHub
parent
dda740fd2d
commit
cd55dfcb3e
@ -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(
|
||||
{},
|
||||
|
||||
@ -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(
|
||||
|
||||
@ -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' },
|
||||
]);
|
||||
});
|
||||
});
|
||||
|
||||
@ -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<string, any>,
|
||||
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
|
||||
*/
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user