fix(core): sort node names for module resolution

This commit is contained in:
Jonathan Cammisuli 2020-01-03 13:27:19 -05:00 committed by Victor Savkin
parent 3d05bf08d0
commit dddc1b1e6c
10 changed files with 181 additions and 58 deletions

View File

@ -25,6 +25,7 @@ import {
readNxJson, readNxJson,
readWorkspaceJson readWorkspaceJson
} from '@nrwl/workspace/src/core/file-utils'; } from '@nrwl/workspace/src/core/file-utils';
import { TargetProjectLocator } from '@nrwl/workspace/src/core/project-graph/build-dependencies/target-project-locator';
type Options = [ type Options = [
{ {
@ -100,6 +101,15 @@ export default createESLintRule<Options, MessageIds>({
} }
const npmScope = (global as any).npmScope; const npmScope = (global as any).npmScope;
const projectGraph = (global as any).projectGraph as ProjectGraph; const projectGraph = (global as any).projectGraph as ProjectGraph;
if (!(global as any).targetProjectLocator) {
(global as any).targetProjectLocator = new TargetProjectLocator(
projectGraph.nodes
);
}
const targetProjectLocator = (global as any)
.targetProjectLocator as TargetProjectLocator;
return { return {
ImportDeclaration(node: TSESTree.ImportDeclaration) { ImportDeclaration(node: TSESTree.ImportDeclaration) {
const imp = (node.source as TSESTree.Literal).value as string; const imp = (node.source as TSESTree.Literal).value as string;
@ -141,6 +151,7 @@ export default createESLintRule<Options, MessageIds>({
// findProjectUsingImport to take care of same prefix // findProjectUsingImport to take care of same prefix
const targetProject = findProjectUsingImport( const targetProject = findProjectUsingImport(
projectGraph, projectGraph,
targetProjectLocator,
sourceFilePath, sourceFilePath,
imp, imp,
npmScope npmScope

View File

@ -10,6 +10,7 @@ import { extname } from 'path';
import enforceModuleBoundaries, { import enforceModuleBoundaries, {
RULE_NAME as enforceModuleBoundariesRuleName RULE_NAME as enforceModuleBoundariesRuleName
} from '../../src/rules/enforce-module-boundaries'; } from '../../src/rules/enforce-module-boundaries';
import { TargetProjectLocator } from '@nrwl/workspace/src/core/project-graph/build-dependencies/target-project-locator';
jest.mock('fs', () => require('memfs').fs); jest.mock('fs', () => require('memfs').fs);
jest.mock('@nrwl/workspace/src/utils/app-root', () => ({ jest.mock('@nrwl/workspace/src/utils/app-root', () => ({
appRootPath: '/root' appRootPath: '/root'
@ -824,6 +825,9 @@ function runRule(
(global as any).projectPath = `${process.cwd()}/proj`; (global as any).projectPath = `${process.cwd()}/proj`;
(global as any).npmScope = 'mycompany'; (global as any).npmScope = 'mycompany';
(global as any).projectGraph = projectGraph; (global as any).projectGraph = projectGraph;
(global as any).targetProjectLocator = new TargetProjectLocator(
projectGraph.nodes
);
const config = { const config = {
...baseConfig, ...baseConfig,

View File

@ -5,7 +5,7 @@ import {
ProjectGraphNodeRecords ProjectGraphNodeRecords
} from '../project-graph-models'; } from '../project-graph-models';
import { TypeScriptImportLocator } from './typescript-import-locator'; import { TypeScriptImportLocator } from './typescript-import-locator';
import { findTargetProjectWithImport } from './find-target-project'; import { TargetProjectLocator } from './target-project-locator';
export function buildExplicitTypeScriptDependencies( export function buildExplicitTypeScriptDependencies(
ctx: ProjectGraphContext, ctx: ProjectGraphContext,
@ -14,17 +14,16 @@ export function buildExplicitTypeScriptDependencies(
fileRead: (s: string) => string fileRead: (s: string) => string
) { ) {
const importLocator = new TypeScriptImportLocator(fileRead); const importLocator = new TypeScriptImportLocator(fileRead);
const targetProjectLocator = new TargetProjectLocator(nodes);
Object.keys(ctx.fileMap).forEach(source => { Object.keys(ctx.fileMap).forEach(source => {
Object.values(ctx.fileMap[source]).forEach(f => { Object.values(ctx.fileMap[source]).forEach(f => {
importLocator.fromFile( importLocator.fromFile(
f.file, f.file,
(importExpr: string, filePath: string, type: DependencyType) => { (importExpr: string, filePath: string, type: DependencyType) => {
const target = findTargetProjectWithImport( const target = targetProjectLocator.findProjectWithImport(
importExpr, importExpr,
f.file, f.file,
ctx.nxJson.npmScope, ctx.nxJson.npmScope
nodes
); );
if (source && target) { if (source && target) {
addDependency(type, source, target); addDependency(type, source, target);

View File

@ -1,25 +0,0 @@
import { resolveModuleByImport } from '../../../utils/typescript';
import { normalizedProjectRoot } from '../../file-utils';
import { ProjectGraphNodeRecords } from '../project-graph-models';
export function findTargetProjectWithImport(
importExpr: string,
filePath: string,
npmScope: string,
nodes: ProjectGraphNodeRecords
) {
const normalizedImportExpr = importExpr.split('#')[0];
const resolvedModule = resolveModuleByImport(normalizedImportExpr, filePath);
return Object.keys(nodes).find(projectName => {
const p = nodes[projectName];
if (resolvedModule && resolvedModule.includes(p.data.root)) {
return true;
} else {
const normalizedRoot = normalizedProjectRoot(p);
const projectImport = `@${npmScope}/${normalizedRoot}`;
return normalizedImportExpr.startsWith(projectImport);
}
});
}

View File

@ -1,7 +1,7 @@
import { fs, vol } from 'memfs'; import { fs, vol } from 'memfs';
import { join } from 'path'; import { join } from 'path';
import { ProjectGraphContext, ProjectGraphNode } from '../../project-graph'; import { ProjectGraphContext, ProjectGraphNode } from '../project-graph-models';
import { findTargetProjectWithImport } from './find-target-project'; import { TargetProjectLocator } from './target-project-locator';
jest.mock('../../../utils/app-root', () => ({ jest.mock('../../../utils/app-root', () => ({
appRootPath: '/root' appRootPath: '/root'
@ -12,6 +12,7 @@ describe('findTargetProjectWithImport', () => {
let ctx: ProjectGraphContext; let ctx: ProjectGraphContext;
let projects: Record<string, ProjectGraphNode>; let projects: Record<string, ProjectGraphNode>;
let fsJson; let fsJson;
let targetProjectLocator: TargetProjectLocator;
beforeEach(() => { beforeEach(() => {
const workspaceJson = { const workspaceJson = {
projects: { projects: {
@ -30,7 +31,10 @@ describe('findTargetProjectWithImport', () => {
paths: { paths: {
'@proj/proj1': ['libs/proj1/index.ts'], '@proj/proj1': ['libs/proj1/index.ts'],
'@proj/my-second-proj': ['libs/proj2/index.ts'], '@proj/my-second-proj': ['libs/proj2/index.ts'],
'@proj/project-3': ['libs/proj3/index.ts'] '@proj/project-3': ['libs/proj3/index.ts'],
'@proj/proj123': ['libs/proj123/index.ts'],
'@proj/proj1234': ['libs/proj1234/index.ts'],
'@proj/proj1234-child': ['libs/proj1234-child/index.ts']
} }
} }
}; };
@ -44,7 +48,10 @@ describe('findTargetProjectWithImport', () => {
`, `,
'./libs/proj2/index.ts': `export const a = 2;`, './libs/proj2/index.ts': `export const a = 2;`,
'./libs/proj3/index.ts': `export const a = 3;`, './libs/proj3/index.ts': `export const a = 3;`,
'./libs/proj4/index.ts': `export const a = 4;` './libs/proj4/index.ts': `export const a = 4;`,
'./libs/proj123/index.ts': 'export const a = 5',
'./libs/proj1234/index.ts': 'export const a = 6',
'./libs/proj1234-child/index.ts': 'export const a = 7'
}; };
vol.fromJSON(fsJson, '/root'); vol.fromJSON(fsJson, '/root');
@ -79,15 +86,38 @@ describe('findTargetProjectWithImport', () => {
mtime: 0, mtime: 0,
ext: '.ts' ext: '.ts'
} }
],
proj123: [
{
file: 'libs/proj123/index.ts',
mtime: 0,
ext: '.ts'
}
],
proj1234: [
{
file: 'libs/proj1234/index.ts',
mtime: 0,
ext: '.ts'
}
],
'proj1234-child': [
{
file: 'libs/proj1234-child/index.ts',
mtime: 0,
ext: '.ts'
}
] ]
} }
}; };
// these projects should be in a random order
projects = { projects = {
proj1: { proj4: {
name: 'proj1', name: 'proj4',
type: 'lib', type: 'lib',
data: { data: {
root: 'libs/proj1', root: 'libs/proj4',
files: [] files: []
} }
}, },
@ -99,6 +129,22 @@ describe('findTargetProjectWithImport', () => {
files: [] files: []
} }
}, },
proj1: {
name: 'proj1',
type: 'lib',
data: {
root: 'libs/proj1',
files: []
}
},
proj1234: {
name: 'proj1234',
type: 'lib',
data: {
root: 'libs/proj1234',
files: []
}
},
proj3: { proj3: {
name: 'proj3', name: 'proj3',
type: 'lib', type: 'lib',
@ -107,41 +153,70 @@ describe('findTargetProjectWithImport', () => {
files: [] files: []
} }
}, },
proj4: { proj123: {
name: 'proj4', name: 'proj123',
type: 'lib', type: 'lib',
data: { data: {
root: 'libs/proj4', root: 'libs/proj123',
files: []
}
},
'proj1234-child': {
name: 'proj1234-child',
type: 'lib',
data: {
root: 'libs/proj1234-child',
files: [] files: []
} }
} }
}; };
targetProjectLocator = new TargetProjectLocator(projects);
}); });
it('should be able to resolve a module by using tsConfig paths', () => { it('should be able to resolve a module by using tsConfig paths', () => {
const proj2 = findTargetProjectWithImport( const proj2 = targetProjectLocator.findProjectWithImport(
'@proj/my-second-proj', '@proj/my-second-proj',
'libs/proj1/index.ts', 'libs/proj1/index.ts',
ctx.nxJson.npmScope, ctx.nxJson.npmScope
projects
); );
const proj3 = findTargetProjectWithImport( const proj3 = targetProjectLocator.findProjectWithImport(
'@proj/project-3', '@proj/project-3',
'libs/proj1/index.ts', 'libs/proj1/index.ts',
ctx.nxJson.npmScope, ctx.nxJson.npmScope
projects
); );
expect(proj2).toEqual('proj2'); expect(proj2).toEqual('proj2');
expect(proj3).toEqual('proj3'); expect(proj3).toEqual('proj3');
}); });
it('should be able to resolve a module using a normalized path', () => { it('should be able to resolve a module using a normalized path', () => {
const proj4 = findTargetProjectWithImport( const proj4 = targetProjectLocator.findProjectWithImport(
'@proj/proj4', '@proj/proj4',
'libs/proj1/index.ts', 'libs/proj1/index.ts',
ctx.nxJson.npmScope, ctx.nxJson.npmScope
projects
); );
expect(proj4).toEqual('proj4'); expect(proj4).toEqual('proj4');
}); });
it('should be able to resolve paths that have similar names', () => {
const proj = targetProjectLocator.findProjectWithImport(
'@proj/proj123',
'',
ctx.nxJson.npmScope
);
expect(proj).toEqual('proj123');
const childProj = targetProjectLocator.findProjectWithImport(
'@proj/proj1234-child',
'',
ctx.nxJson.npmScope
);
expect(childProj).toEqual('proj1234-child');
const parentProj = targetProjectLocator.findProjectWithImport(
'@proj/proj1234',
'',
ctx.nxJson.npmScope
);
expect(parentProj).toEqual('proj1234');
});
}); });

View File

@ -0,0 +1,45 @@
import { resolveModuleByImport } from '../../../utils/typescript';
import { normalizedProjectRoot } from '../../file-utils';
import { ProjectGraphNodeRecords } from '../project-graph-models';
export class TargetProjectLocator {
private sortedNodeNames = [];
constructor(private nodes: ProjectGraphNodeRecords) {
this.sortedNodeNames = Object.keys(nodes).sort((a, b) => {
if (!nodes[a].data.root) return -1;
if (!nodes[b].data.root) return -1;
return nodes[a].data.root.length > nodes[b].data.root.length ? -1 : 1;
});
}
findProjectWithImport(
importExpr: string,
filePath: string,
npmScope: string
) {
const normalizedImportExpr = importExpr.split('#')[0];
const resolvedModule = resolveModuleByImport(
normalizedImportExpr,
filePath
);
return this.sortedNodeNames.find(projectName => {
const p = this.nodes[projectName];
// If there is no root in the data object, it's not a nx project (ie. npm module)
if (!p.data.root) {
return false;
}
if (resolvedModule && resolvedModule.startsWith(p.data.root)) {
return true;
} else {
const normalizedRoot = normalizedProjectRoot(p);
const projectImport = `@${npmScope}/${normalizedRoot}`;
return normalizedImportExpr.startsWith(projectImport);
}
});
}
}

View File

@ -8,6 +8,7 @@ import {
ProjectType ProjectType
} from '../core/project-graph'; } from '../core/project-graph';
import { Rule } from './nxEnforceModuleBoundariesRule'; import { Rule } from './nxEnforceModuleBoundariesRule';
import { TargetProjectLocator } from '../core/project-graph/build-dependencies/target-project-locator';
jest.mock('fs', () => require('memfs').fs); jest.mock('fs', () => require('memfs').fs);
jest.mock('../utils/app-root', () => ({ appRootPath: '/root' })); jest.mock('../utils/app-root', () => ({ appRootPath: '/root' }));
@ -821,7 +822,8 @@ function runRule(
options, options,
`${process.cwd()}/proj`, `${process.cwd()}/proj`,
'mycompany', 'mycompany',
projectGraph projectGraph,
new TargetProjectLocator(projectGraph.nodes)
); );
return rule.apply(sourceFile); return rule.apply(sourceFile);
} }

View File

@ -28,13 +28,15 @@ import {
readNxJson, readNxJson,
readWorkspaceJson readWorkspaceJson
} from '@nrwl/workspace/src/core/file-utils'; } from '@nrwl/workspace/src/core/file-utils';
import { TargetProjectLocator } from '../core/project-graph/build-dependencies/target-project-locator';
export class Rule extends Lint.Rules.AbstractRule { export class Rule extends Lint.Rules.AbstractRule {
constructor( constructor(
options: IOptions, options: IOptions,
private readonly projectPath?: string, private readonly projectPath?: string,
private readonly npmScope?: string, private readonly npmScope?: string,
private readonly projectGraph?: ProjectGraph private readonly projectGraph?: ProjectGraph,
private readonly targetProjectLocator?: TargetProjectLocator
) { ) {
super(options); super(options);
@ -51,6 +53,13 @@ export class Rule extends Lint.Rules.AbstractRule {
} }
this.npmScope = (global as any).npmScope; this.npmScope = (global as any).npmScope;
this.projectGraph = (global as any).projectGraph; this.projectGraph = (global as any).projectGraph;
if (!(global as any).targetProjectLocator) {
(global as any).targetProjectLocator = new TargetProjectLocator(
this.projectGraph.nodes
);
}
this.targetProjectLocator = (global as any).targetProjectLocator;
} }
} }
@ -61,7 +70,8 @@ export class Rule extends Lint.Rules.AbstractRule {
this.getOptions(), this.getOptions(),
this.projectPath, this.projectPath,
this.npmScope, this.npmScope,
this.projectGraph this.projectGraph,
this.targetProjectLocator
) )
); );
} }
@ -76,7 +86,8 @@ class EnforceModuleBoundariesWalker extends Lint.RuleWalker {
options: IOptions, options: IOptions,
private readonly projectPath: string, private readonly projectPath: string,
private readonly npmScope: string, private readonly npmScope: string,
private readonly projectGraph: ProjectGraph private readonly projectGraph: ProjectGraph,
private readonly targetProjectLocator: TargetProjectLocator
) { ) {
super(sourceFile, options); super(sourceFile, options);
@ -132,6 +143,7 @@ class EnforceModuleBoundariesWalker extends Lint.RuleWalker {
// findProjectUsingImport to take care of same prefix // findProjectUsingImport to take care of same prefix
const targetProject = findProjectUsingImport( const targetProject = findProjectUsingImport(
this.projectGraph, this.projectGraph,
this.targetProjectLocator,
filePath, filePath,
imp, imp,
this.npmScope this.npmScope

View File

@ -7,7 +7,7 @@ import {
ProjectGraphDependency, ProjectGraphDependency,
ProjectGraphNode ProjectGraphNode
} from '../core/project-graph'; } from '../core/project-graph';
import { findTargetProjectWithImport } from '../core/project-graph/build-dependencies/find-target-project'; import { TargetProjectLocator } from '../core/project-graph/build-dependencies/target-project-locator';
export type Deps = { [projectName: string]: ProjectGraphDependency[] }; export type Deps = { [projectName: string]: ProjectGraphDependency[] };
export type DepConstraint = { export type DepConstraint = {
@ -134,15 +134,15 @@ export function isAbsoluteImportIntoAnotherProject(imp: string) {
export function findProjectUsingImport( export function findProjectUsingImport(
projectGraph: ProjectGraph, projectGraph: ProjectGraph,
targetProjectLocator: TargetProjectLocator,
filePath: string, filePath: string,
imp: string, imp: string,
npmScope: string npmScope: string
) { ) {
const target = findTargetProjectWithImport( const target = targetProjectLocator.findProjectWithImport(
imp, imp,
filePath, filePath,
npmScope, npmScope
projectGraph.nodes
); );
return projectGraph.nodes[target]; return projectGraph.nodes[target];
} }

View File

@ -38,7 +38,7 @@ export function resolveModuleByImport(importExpr: string, filePath: string) {
if (!resolvedModule) { if (!resolvedModule) {
return; return;
} else { } else {
return resolvedModule.resolvedFileName.replace(appRootPath, ''); return resolvedModule.resolvedFileName.replace(`${appRootPath}/`, '');
} }
} }