From 5ddadc42f55564cc5b96c6a27d57992996e7de2f Mon Sep 17 00:00:00 2001 From: timdeschryver <28659384+timdeschryver@users.noreply.github.com> Date: Sun, 10 Jan 2021 10:12:10 +0100 Subject: [PATCH] perf(schematics): speed up create effect migration Exclude node_modules from being visited --- .../src/create-effect-migration/index.spec.ts | 18 ++++--- .../src/create-effect-migration/index.ts | 53 +++++++++---------- 2 files changed, 36 insertions(+), 35 deletions(-) diff --git a/modules/schematics/src/create-effect-migration/index.spec.ts b/modules/schematics/src/create-effect-migration/index.spec.ts index 70c90424b0..f4a1c269b0 100644 --- a/modules/schematics/src/create-effect-migration/index.spec.ts +++ b/modules/schematics/src/create-effect-migration/index.spec.ts @@ -35,7 +35,7 @@ describe('Creator migration', () => { @Injectable() export class SomeEffectsClass { constructor(private actions$: Actions) {} - ** + foo$ = createEffect(() => this.actions$.pipe( ofType(AuthActions.login), tap(action => console.log(action)) @@ -63,7 +63,7 @@ describe('Creator migration', () => { @Injectable() export class SomeEffectsClass { constructor(private actions$: Actions) {} - ** + bar$ = createEffect(() => this.actions$.pipe( ofType(AuthActions.login, AuthActions.logout), tap(action => console.log(action)) @@ -91,7 +91,7 @@ describe('Creator migration', () => { @Injectable() export class SomeEffectsClass { constructor(private actions$: Actions) {} - ** + baz$ = createEffect(() => ({ debounce = 300, scheduler = asyncScheduler } = {}) => this.actions$.pipe( ofType(login), tap(action => console.log(action)) @@ -126,14 +126,14 @@ describe('Creator migration', () => { @Injectable() export class SomeEffectsClass { constructor(private actions$: Actions) {} - ** + @Log() login$ = createEffect(() => this.actions$.pipe( ofType('LOGIN'), map(() => ({ type: 'LOGGED_IN' })) )); + @Log() - ** logout$ = createEffect(() => this.actions$.pipe( ofType('LOGOUT'), map(() => ({ type: 'LOGGED_OUT' })) @@ -182,7 +182,7 @@ describe('Creator migration', () => { import { Actions, createEffect, ofType } from '@ngrx/effects'; @Injectable() export class SomeEffectsClass { - ** + logout$ = createEffect(() => this.actions$.pipe( ofType('LOGOUT'), map(() => ({ type: 'LOGGED_OUT' })) @@ -221,7 +221,9 @@ describe('Creator migration', () => { const actual = tree.readContent(effectPath); - // ** for indentation because empty lines are formatted on auto-save - expect(actual).toBe(expected.replace(/\*\*/g, ' ')); + const removeEmptyLines = (value: string) => + value.replace(/^\s*$(?:\r\n?|\n)/gm, ''); + + expect(removeEmptyLines(actual)).toBe(removeEmptyLines(expected)); } }); diff --git a/modules/schematics/src/create-effect-migration/index.ts b/modules/schematics/src/create-effect-migration/index.ts index ec2fbb5c05..798a3ed3a3 100644 --- a/modules/schematics/src/create-effect-migration/index.ts +++ b/modules/schematics/src/create-effect-migration/index.ts @@ -6,25 +6,12 @@ import { RemoveChange, replaceImport, commitChanges, + visitTSSourceFiles, } from '@ngrx/schematics/schematics-core'; export function migrateToCreators(): Rule { - return (host: Tree) => - host.visit((path) => { - if (!path.endsWith('.ts')) { - return; - } - - const sourceFile = ts.createSourceFile( - path, - host.read(path)!.toString(), - ts.ScriptTarget.Latest - ); - - if (sourceFile.isDeclarationFile) { - return; - } - + return (tree: Tree) => { + visitTSSourceFiles(tree, (sourceFile) => { const effectsPerClass = sourceFile.statements .filter(ts.isClassDeclaration) .map((clas) => @@ -42,37 +29,50 @@ export function migrateToCreators(): Rule { [] ); - const createEffectsChanges = replaceEffectDecorators(host, path, effects); + const createEffectsChanges = replaceEffectDecorators( + tree, + sourceFile, + effects + ); const importChanges = replaceImport( sourceFile, - path, + sourceFile.fileName as Path, '@ngrx/effects', 'Effect', 'createEffect' ); - return commitChanges(host, sourceFile.fileName, [ + commitChanges(tree, sourceFile.fileName, [ ...importChanges, ...createEffectsChanges, ]); }); + }; } function replaceEffectDecorators( host: Tree, - path: Path, + sourceFile: ts.SourceFile, effects: ts.PropertyDeclaration[] ) { const inserts = effects .filter((effect) => !!effect.initializer) .map((effect) => { const decorator = (effect.decorators || []).find(isEffectDecorator)!; - const effectArguments = getDispatchProperties(host, path, decorator); + const effectArguments = getDispatchProperties( + host, + sourceFile.text, + decorator + ); const end = effectArguments ? `, ${effectArguments})` : ')'; return [ - new InsertChange(path, effect.initializer!.pos, ' createEffect(() =>'), - new InsertChange(path, effect.initializer!.end, end), + new InsertChange( + sourceFile.fileName, + effect.initializer!.pos, + ' createEffect(() =>' + ), + new InsertChange(sourceFile.fileName, effect.initializer!.end, end), ]; }) .reduce((acc, inserts) => acc.concat(inserts), []); @@ -84,7 +84,7 @@ function replaceEffectDecorators( const effectDecorators = decorators!.filter(isEffectDecorator); return effectDecorators.map((decorator) => { return new RemoveChange( - path, + sourceFile.fileName, decorator.expression.pos - 1, // also get the @ sign decorator.expression.end ); @@ -105,7 +105,7 @@ function isEffectDecorator(decorator: ts.Decorator) { function getDispatchProperties( host: Tree, - path: Path, + fileContent: string, decorator: ts.Decorator ) { if (!decorator.expression || !ts.isCallExpression(decorator.expression)) { @@ -113,8 +113,7 @@ function getDispatchProperties( } // just copy the effect properties - const content = host.read(path)!.toString('utf8'); - const args = content + const args = fileContent .substring( decorator.expression.arguments.pos, decorator.expression.arguments.end