Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

perf: fine tune schematics #1925

Merged
merged 1 commit into from
Jun 6, 2019
Merged

perf: fine tune schematics #1925

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions modules/data/schematics-core/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ export {
ReplaceChange,
createReplaceChange,
createChangeRecorder,
commitChanges,
} from './utility/change';

export { AppConfig, getWorkspace, getWorkspacePath } from './utility/config';
Expand Down
2 changes: 1 addition & 1 deletion modules/data/schematics-core/utility/ast-utils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -687,7 +687,7 @@ export function replaceImport(
})
.filter(({ hit }) => hit)
.map(({ specifier, text }) =>
createReplaceChange(sourceFile, path, specifier!, text!, importToBe)
createReplaceChange(sourceFile, specifier!, text!, importToBe)
);

return changes;
Expand Down
18 changes: 16 additions & 2 deletions modules/data/schematics-core/utility/change.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -136,12 +136,16 @@ export class ReplaceChange implements Change {

export function createReplaceChange(
sourceFile: ts.SourceFile,
path: Path,
node: ts.Node,
oldText: string,
newText: string
): ReplaceChange {
return new ReplaceChange(path, node.getStart(sourceFile), oldText, newText);
return new ReplaceChange(
sourceFile.fileName,
node.getStart(sourceFile),
oldText,
newText
);
}

export function createChangeRecorder(
Expand All @@ -162,3 +166,13 @@ export function createChangeRecorder(
}
return recorder;
}

export function commitChanges(tree: Tree, path: string, changes: Change[]) {
if (changes.length === 0) {
return false;
}

const recorder = createChangeRecorder(tree, path, changes);
tree.commitUpdate(recorder);
return true;
}
43 changes: 27 additions & 16 deletions modules/data/schematics-core/utility/visit-utils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import * as ts from 'typescript';
import { Tree } from '@angular-devkit/schematics';
import { Tree, DirEntry } from '@angular-devkit/schematics';

export function visitTSSourceFiles<Result = void>(
tree: Tree,
Expand All @@ -10,24 +10,35 @@ export function visitTSSourceFiles<Result = void>(
) => Result | undefined
): Result | undefined {
let result: Result | undefined = undefined;
for (const sourceFile of visit(tree.root)) {
result = visitor(sourceFile, tree, result);
}

tree.visit(path => {
if (!path.endsWith('.ts')) {
return;
}

const sourceFile = ts.createSourceFile(
path,
tree.read(path)!.toString(),
ts.ScriptTarget.Latest
);
return result;
}

if (sourceFile.isDeclarationFile) {
return;
function* visit(directory: DirEntry): IterableIterator<ts.SourceFile> {
for (const path of directory.subfiles) {
if (path.endsWith('.ts') && !path.endsWith('.d.ts')) {
const entry = directory.file(path);
if (entry) {
const content = entry.content;
const source = ts.createSourceFile(
entry.path,
content.toString().replace(/^\uFEFF/, ''),
ts.ScriptTarget.Latest,
true
);
yield source;
}
}
}

result = visitor(sourceFile, tree, result);
});
for (const path of directory.subdirs) {
if (path === 'node_modules') {
continue;
}

return result;
yield* visit(directory.dir(path));
}
}
14 changes: 4 additions & 10 deletions modules/data/schematics/ng-add/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ import {
createChangeRecorder,
} from '@ngrx/data/schematics-core';
import { Schema as EntityDataOptions } from './schema';
import { Path } from '@angular-devkit/core';

function addNgRxDataToPackageJson() {
return (host: Tree, context: SchematicContext) => {
Expand Down Expand Up @@ -133,9 +132,9 @@ function renameNgrxDataModule(options: EntityDataOptions) {
}

const changes = [
...findNgrxDataImports(sourceFile, path, ngrxDataImports),
...findNgrxDataImportDeclarations(sourceFile, path, ngrxDataImports),
...findNgrxDataReplacements(sourceFile, path),
...findNgrxDataImports(sourceFile, ngrxDataImports),
...findNgrxDataImportDeclarations(sourceFile, ngrxDataImports),
...findNgrxDataReplacements(sourceFile),
];

if (changes.length === 0) {
Expand All @@ -150,13 +149,11 @@ function renameNgrxDataModule(options: EntityDataOptions) {

function findNgrxDataImports(
sourceFile: ts.SourceFile,
path: Path,
imports: ts.ImportDeclaration[]
) {
const changes = imports.map(specifier =>
createReplaceChange(
sourceFile,
path,
specifier.moduleSpecifier,
"'ngrx-data'",
"'@ngrx/data'"
Expand All @@ -168,7 +165,6 @@ function findNgrxDataImports(

function findNgrxDataImportDeclarations(
sourceFile: ts.SourceFile,
path: Path,
imports: ts.ImportDeclaration[]
) {
const changes = imports
Expand Down Expand Up @@ -198,7 +194,6 @@ function findNgrxDataImportDeclarations(
.map(({ specifier, text }) =>
createReplaceChange(
sourceFile,
path,
specifier!,
text!,
(renames as any)[text!]
Expand All @@ -208,7 +203,7 @@ function findNgrxDataImportDeclarations(
return changes;
}

function findNgrxDataReplacements(sourceFile: ts.SourceFile, path: Path) {
function findNgrxDataReplacements(sourceFile: ts.SourceFile) {
const renameKeys = Object.keys(renames);
let changes: ReplaceChange[] = [];
ts.forEachChild(sourceFile, node => find(node, changes));
Expand Down Expand Up @@ -252,7 +247,6 @@ function findNgrxDataReplacements(sourceFile: ts.SourceFile, path: Path) {
changes.push(
createReplaceChange(
sourceFile,
path,
change.node,
change.text,
(renames as any)[change.text]
Expand Down
1 change: 1 addition & 0 deletions modules/data/tsconfig-build.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
"noEmitOnError": false,
"noImplicitAny": true,
"noImplicitReturns": true,
"downlevelIteration": true,
"outDir": "../../dist/packages/data",
"paths": {
"@ngrx/store": ["../../dist/packages/store"],
Expand Down
1 change: 1 addition & 0 deletions modules/effects/schematics-core/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ export {
ReplaceChange,
createReplaceChange,
createChangeRecorder,
commitChanges,
} from './utility/change';

export { AppConfig, getWorkspace, getWorkspacePath } from './utility/config';
Expand Down
2 changes: 1 addition & 1 deletion modules/effects/schematics-core/utility/ast-utils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -687,7 +687,7 @@ export function replaceImport(
})
.filter(({ hit }) => hit)
.map(({ specifier, text }) =>
createReplaceChange(sourceFile, path, specifier!, text!, importToBe)
createReplaceChange(sourceFile, specifier!, text!, importToBe)
);

return changes;
Expand Down
18 changes: 16 additions & 2 deletions modules/effects/schematics-core/utility/change.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -136,12 +136,16 @@ export class ReplaceChange implements Change {

export function createReplaceChange(
sourceFile: ts.SourceFile,
path: Path,
node: ts.Node,
oldText: string,
newText: string
): ReplaceChange {
return new ReplaceChange(path, node.getStart(sourceFile), oldText, newText);
return new ReplaceChange(
sourceFile.fileName,
node.getStart(sourceFile),
oldText,
newText
);
}

export function createChangeRecorder(
Expand All @@ -162,3 +166,13 @@ export function createChangeRecorder(
}
return recorder;
}

export function commitChanges(tree: Tree, path: string, changes: Change[]) {
if (changes.length === 0) {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only commit the file when there are changes, I forgot this in some of the v8 migrations making them slow (plus there was a information message on every file changes, even node_modules, which might confuse people).

return false;
}

const recorder = createChangeRecorder(tree, path, changes);
tree.commitUpdate(recorder);
return true;
}
43 changes: 27 additions & 16 deletions modules/effects/schematics-core/utility/visit-utils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import * as ts from 'typescript';
import { Tree } from '@angular-devkit/schematics';
import { Tree, DirEntry } from '@angular-devkit/schematics';

export function visitTSSourceFiles<Result = void>(
tree: Tree,
Expand All @@ -10,24 +10,35 @@ export function visitTSSourceFiles<Result = void>(
) => Result | undefined
): Result | undefined {
let result: Result | undefined = undefined;
for (const sourceFile of visit(tree.root)) {
result = visitor(sourceFile, tree, result);
}

tree.visit(path => {
if (!path.endsWith('.ts')) {
return;
}

const sourceFile = ts.createSourceFile(
path,
tree.read(path)!.toString(),
ts.ScriptTarget.Latest
);
return result;
}

if (sourceFile.isDeclarationFile) {
return;
function* visit(directory: DirEntry): IterableIterator<ts.SourceFile> {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't use tree.visit for more flexibility (like ignore node_modules).
Inspiration from https://github.com/angular/angular-cli/blob/576119f2cab6a53f0bc67026b723fffbe8407258/packages/schematics/angular/migrations/update-8/update-lazy-module-paths.ts. I like this solution the most as we break out the node_modules folder as fast as possible.

Another option is to use the ignore package, like the nx workspace does - https://github.com/nrwl/nx/blob/3af31f1cd996db2f225773eb1888bd109efb9f76/packages/schematics/migrations/update-8-0-0/update-8-0-0.ts#L176

Or to simple check if node_modules is in the path.

for (const path of directory.subfiles) {
if (path.endsWith('.ts') && !path.endsWith('.d.ts')) {
const entry = directory.file(path);
if (entry) {
const content = entry.content;
const source = ts.createSourceFile(
entry.path,
content.toString().replace(/^\uFEFF/, ''),
ts.ScriptTarget.Latest,
true
);
yield source;
}
}
}

result = visitor(sourceFile, tree, result);
});
for (const path of directory.subdirs) {
if (path === 'node_modules') {
continue;
}

return result;
yield* visit(directory.dir(path));
}
}
1 change: 1 addition & 0 deletions modules/effects/tsconfig-build.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
"noEmitOnError": false,
"noImplicitAny": true,
"noImplicitReturns": true,
"downlevelIteration": true,
"outDir": "../../dist/packages/effects",
"paths": {
"@ngrx/store": ["../../dist/packages/store"]
Expand Down
1 change: 1 addition & 0 deletions modules/entity/schematics-core/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ export {
ReplaceChange,
createReplaceChange,
createChangeRecorder,
commitChanges,
} from './utility/change';

export { AppConfig, getWorkspace, getWorkspacePath } from './utility/config';
Expand Down
2 changes: 1 addition & 1 deletion modules/entity/schematics-core/utility/ast-utils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -687,7 +687,7 @@ export function replaceImport(
})
.filter(({ hit }) => hit)
.map(({ specifier, text }) =>
createReplaceChange(sourceFile, path, specifier!, text!, importToBe)
createReplaceChange(sourceFile, specifier!, text!, importToBe)
);

return changes;
Expand Down
18 changes: 16 additions & 2 deletions modules/entity/schematics-core/utility/change.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -136,12 +136,16 @@ export class ReplaceChange implements Change {

export function createReplaceChange(
sourceFile: ts.SourceFile,
path: Path,
node: ts.Node,
oldText: string,
newText: string
): ReplaceChange {
return new ReplaceChange(path, node.getStart(sourceFile), oldText, newText);
return new ReplaceChange(
sourceFile.fileName,
node.getStart(sourceFile),
oldText,
newText
);
}

export function createChangeRecorder(
Expand All @@ -162,3 +166,13 @@ export function createChangeRecorder(
}
return recorder;
}

export function commitChanges(tree: Tree, path: string, changes: Change[]) {
if (changes.length === 0) {
return false;
}

const recorder = createChangeRecorder(tree, path, changes);
tree.commitUpdate(recorder);
return true;
}
Loading