From 93ecf24bdadfa7ae28cf802b4f3193390858db90 Mon Sep 17 00:00:00 2001 From: vsavkin Date: Thu, 15 Mar 2018 14:05:25 -0400 Subject: [PATCH] feat(schematics): cache the dependency graph to speed up "ng lint" --- .../src/command-line/affected-apps.spec.ts | 60 ++++++++++++++++ .../src/command-line/affected-apps.ts | 6 +- .../schematics/src/command-line/shared.ts | 69 ++++++++++++++++++- .../nxEnforceModuleBoundariesRule.spec.ts | 50 +++++++++++++- .../tslint/nxEnforceModuleBoundariesRule.ts | 34 ++++++--- 5 files changed, 205 insertions(+), 14 deletions(-) diff --git a/packages/schematics/src/command-line/affected-apps.spec.ts b/packages/schematics/src/command-line/affected-apps.spec.ts index fc4218054d092..0005ab11a81ee 100644 --- a/packages/schematics/src/command-line/affected-apps.spec.ts +++ b/packages/schematics/src/command-line/affected-apps.spec.ts @@ -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', () => { diff --git a/packages/schematics/src/command-line/affected-apps.ts b/packages/schematics/src/command-line/affected-apps.ts index c626bef6fe4a6..b027b1b025c6b 100644 --- a/packages/schematics/src/command-line/affected-apps.ts +++ b/packages/schematics/src/command-line/affected-apps.ts @@ -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}); + } } } diff --git a/packages/schematics/src/command-line/shared.ts b/packages/schematics/src/command-line/shared.ts index cc7b00d63aa98..61a9c8302a4bc 100644 --- a/packages/schematics/src/command-line/shared.ts +++ b/packages/schematics/src/command-line/shared.ts @@ -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 = []; @@ -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; + } +} \ No newline at end of file diff --git a/packages/schematics/src/tslint/nxEnforceModuleBoundariesRule.spec.ts b/packages/schematics/src/tslint/nxEnforceModuleBoundariesRule.spec.ts index fd5421a6965a3..3f6d70d39959b 100644 --- a/packages/schematics/src/tslint/nxEnforceModuleBoundariesRule.spec.ts +++ b/packages/schematics/src/tslint/nxEnforceModuleBoundariesRule.spec.ts @@ -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 = [ { @@ -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', @@ -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', () => { diff --git a/packages/schematics/src/tslint/nxEnforceModuleBoundariesRule.ts b/packages/schematics/src/tslint/nxEnforceModuleBoundariesRule.ts index 24d373445a5f4..2148fb9872a32 100644 --- a/packages/schematics/src/tslint/nxEnforceModuleBoundariesRule.ts +++ b/packages/schematics/src/tslint/nxEnforceModuleBoundariesRule.ts @@ -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( @@ -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; } } @@ -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}`) : []; @@ -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) { @@ -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; } @@ -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) {