Skip to content

Commit

Permalink
feat(schematics): cache the dependency graph to speed up "ng lint"
Browse files Browse the repository at this point in the history
  • Loading branch information
vsavkin committed Mar 16, 2018
1 parent 74d920a commit 93ecf24
Show file tree
Hide file tree
Showing 5 changed files with 205 additions and 14 deletions.
60 changes: 60 additions & 0 deletions packages/schematics/src/command-line/affected-apps.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,66 @@ describe('Calculates Dependencies Between Apps and Libs', () => {

expect(deps).toEqual({ aa: [{projectName: 'aa/bb', type: DependencyType.es6Import}], 'aa/bb': [] });
});

it('should not add the same dependency twice', () => {
const deps = dependencies(
'nrwl',
[
{
name: 'aa',
root: '',
files: ['aa.ts'],
tags: [],
type: ProjectType.app
},
{
name: 'bb',
root: '',
files: ['bb.ts'],
tags: [],
type: ProjectType.app
}
],
file => {
switch (file) {
case 'aa.ts':
return `
import '@nrwl/bb/bb'
import '@nrwl/bb/bb'
`;
case 'bb.ts':
return '';
}
}
);

expect(deps).toEqual({ aa: [{projectName: 'bb', type: DependencyType.es6Import}], bb: []});
});

it('should not add a dependency on self', () => {
const deps = dependencies(
'nrwl',
[
{
name: 'aa',
root: '',
files: ['aa.ts'],
tags: [],
type: ProjectType.app
}
],
file => {
switch (file) {
case 'aa.ts':
return `
import '@nrwl/aa/aa'
`;
}
}
);

expect(deps).toEqual({ aa: []});
});
});

describe('affectedApps', () => {
Expand Down
6 changes: 5 additions & 1 deletion packages/schematics/src/command-line/affected-apps.ts
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,11 @@ class Deps {
)[0];

if (matchingProject) {
this.deps[projectName].push({projectName: matchingProject, type: depType});
const alreadyHasDep = this.deps[projectName].some(p => p.projectName === matchingProject && p.type === depType);
const depOnSelf = projectName === matchingProject;
if (!alreadyHasDep && !depOnSelf) {
this.deps[projectName].push({projectName: matchingProject, type: depType});
}
}
}

Expand Down
69 changes: 68 additions & 1 deletion packages/schematics/src/command-line/shared.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
import { execSync } from 'child_process';
import * as path from 'path';
import {affectedApps, ProjectType, touchedProjects} from './affected-apps';
import {affectedApps, ProjectNode, ProjectType, touchedProjects} from './affected-apps';
import * as fs from 'fs';
import {dependencies, Dependency} from '@nrwl/schematics/src/command-line/affected-apps';
import {readFileSync, statSync} from "fs";

export function parseFiles(args: string[]): { files: string[]; rest: string[] } {
let unnamed = [];
Expand Down Expand Up @@ -94,6 +96,71 @@ function allFilesInDir(dirName: string): string[] {
return res;
}

export function readDependencies(npmScope: string, projectNodes: ProjectNode[]): { [projectName: string]: Dependency[] } {
const m = lastModifiedAmongProjectFiles();
if (!directoryExists('./dist')) {
fs.mkdirSync('./dist');
}
if (!fileExists('./dist/nxdeps.json') || m > mtime('./dist/nxdeps.json')) {
const deps = dependencies(npmScope, projectNodes, f => fs.readFileSync(f, 'UTF-8'));
fs.writeFileSync('./dist/nxdeps.json', JSON.stringify(deps, null, 2), 'UTF-8');
return deps;
} else {
return JSON.parse(fs.readFileSync('./dist/nxdeps.json', 'UTF-8'));
}
}

export function lastModifiedAmongProjectFiles() {
return [
recursiveMtime('libs'),
recursiveMtime('apps'),
mtime('.angular-cli.json'),
mtime('tslint.json'),
mtime('package.json')
].reduce((a, b) => a > b ? a : b, 0);
}

function recursiveMtime(dirName: string) {
let res = mtime(dirName);
fs.readdirSync(dirName).forEach(c => {
const child = path.join(dirName, c);
try {
if (!fs.statSync(child).isDirectory()) {
const c = mtime(child);
if (c > res) {
res = c;
}
} else if (fs.statSync(child).isDirectory()) {
const c = recursiveMtime(child);
if (c > res) {
res = c;
}
}
} catch (e) {}
});
return res;
}

function mtime(f: string): number {
return fs.fstatSync(fs.openSync(f, 'r')).mtime.getTime();
}

function normalizePath(file: string): string {
return file.split(path.sep).join('/');
}

function directoryExists(filePath: string): boolean {
try {
return statSync(filePath).isDirectory();
} catch (err) {
return false;
}
}

function fileExists(filePath: string): boolean {
try {
return statSync(filePath).isFile();
} catch (err) {
return false;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,41 @@ describe('Enforce Module Boundaries', () => {
expect(failures.length).toEqual(0);
});

it('should handle multiple projects starting with the same prefix properly', () => {
const failures = runRule(
{},
`${process.cwd()}/proj/apps/myapp/src/main.ts`,
`
import '@mycompany/myapp2/mylib';
`,
[
{
name: 'myapp',
root: 'libs/myapp/src',
type: ProjectType.app,
tags: [],
files: [`apps/myapp/src/main.ts`, `apps/myapp/blah.ts`]
},
{
name: 'myapp2',
root: 'libs/myapp2/src',
type: ProjectType.app,
tags: [],
files: []
},
{
name: 'myapp2/mylib',
root: 'libs/myapp2/mylib/src',
type: ProjectType.lib,
tags: [],
files: ['libs/myapp2/mylib/src/index.ts']
}
]
);

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

describe('depConstraints', () => {
const projectNodes = [
{
Expand Down Expand Up @@ -244,7 +279,10 @@ describe('Enforce Module Boundaries', () => {
const failures = runRule(
{},
`${process.cwd()}/proj/libs/mylib/src/main.ts`,
'import "@mycompany/other/blah"',
`
import "@mycompany/other/blah"
import "@mycompany/other/sublib/blah"
`,
[
{
name: 'mylib',
Expand All @@ -259,12 +297,22 @@ describe('Enforce Module Boundaries', () => {
type: ProjectType.lib,
tags: [],
files: [`libs/other/blah.ts`]
},
{
name: 'other/sublib',
root: 'libs/other/sublib/src',
type: ProjectType.lib,
tags: [],
files: [`libs/other/sublib/blah.ts`]
}
]
);
expect(failures[0].getFailure()).toEqual(
'deep imports into libraries are forbidden'
);
expect(failures[1].getFailure()).toEqual(
'deep imports into libraries are forbidden'
);
});

it('should error on importing a lazy-loaded library', () => {
Expand Down
34 changes: 23 additions & 11 deletions packages/schematics/src/tslint/nxEnforceModuleBoundariesRule.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@ import { IOptions } from 'tslint';
import * as ts from 'typescript';
import { readFileSync } from 'fs';
import * as appRoot from 'app-root-path';
import {getProjectNodes} from '../command-line/shared';
import {dependencies, Dependency, DependencyType, ProjectNode, ProjectType} from '../command-line/affected-apps';
import {getProjectNodes, readDependencies} from '../command-line/shared';
import {Dependency, DependencyType, ProjectNode, ProjectType} from '../command-line/affected-apps';

export class Rule extends Lint.Rules.AbstractRule {
constructor(
Expand All @@ -18,10 +18,15 @@ export class Rule extends Lint.Rules.AbstractRule {
super(options);
if (!projectPath) {
this.projectPath = appRoot.path;
const cliConfig = this.readCliConfig(this.projectPath);
this.npmScope = cliConfig.project.npmScope;
this.projectNodes = getProjectNodes(cliConfig);
this.deps = dependencies(this.npmScope, this.projectNodes, f => readFileSync(f).toString());
if (!(global as any).projectNodes) {
const cliConfig = this.readCliConfig(this.projectPath);
(global as any).npmScope = cliConfig.project.npmScope;
(global as any).projectNodes = getProjectNodes(cliConfig);
(global as any).deps = readDependencies((global as any).npmScope, (global as any).projectNodes);
}
this.npmScope = (global as any).npmScope;
this.projectNodes = (global as any).projectNodes;
this.deps =(global as any).deps;
}
}

Expand Down Expand Up @@ -62,6 +67,12 @@ class EnforceModuleBoundariesWalker extends Lint.RuleWalker {
) {
super(sourceFile, options);

this.projectNodes.sort((a, b) => {
if (!a.name) return -1;
if (!b.name) return -1;
return a.name.length > b.name.length ? -1 : 1;
});

this.allow = Array.isArray(this.getOptions()[0].allow)
? this.getOptions()[0].allow.map(a => `${a}`)
: [];
Expand All @@ -88,9 +99,9 @@ class EnforceModuleBoundariesWalker extends Lint.RuleWalker {

// check constraints between libs and apps
if (imp.startsWith(`@${this.npmScope}/`)) {
const name = imp.split('/')[1];
// we should find the name
const sourceProject = this.findSourceProject();
const targetProject = this.findProjectUsingName(name);
const targetProject = this.findProjectUsingImport(imp); // findProjectUsingImport to take care of same prefix

// something went wrong => return.
if (!sourceProject || !targetProject) {
Expand Down Expand Up @@ -118,7 +129,7 @@ class EnforceModuleBoundariesWalker extends Lint.RuleWalker {
}

// deep imports aren't allowed
if (imp.split('/').length > 2) {
if (imp !== `@${this.npmScope}/${targetProject.name}`) {
this.addFailureAt(node.getStart(), node.getWidth(), 'deep imports into libraries are forbidden');
return;
}
Expand Down Expand Up @@ -202,8 +213,9 @@ class EnforceModuleBoundariesWalker extends Lint.RuleWalker {
return this.projectNodes.filter(n => containsFile(n.files, file))[0];
}

private findProjectUsingName(name: string) {
return this.projectNodes.filter(n => n.name === name)[0];
private findProjectUsingImport(imp: string) {
const unscopedImport = imp.substring(this.npmScope.length + 2);
return this.projectNodes.filter(n => unscopedImport === n.name || unscopedImport.startsWith(`${n.name}/`))[0];
}

private isAbsoluteImportIntoAnotherProject(imp: string) {
Expand Down

0 comments on commit 93ecf24

Please sign in to comment.