Skip to content

Commit 2f5dcb4

Browse files
perf(schematics): speed up create effect migration (#2873)
Exclude node_modules from being visited
1 parent c3ac252 commit 2f5dcb4

File tree

2 files changed

+36
-35
lines changed

2 files changed

+36
-35
lines changed

modules/schematics/src/create-effect-migration/index.spec.ts

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ describe('Creator migration', () => {
3535
@Injectable()
3636
export class SomeEffectsClass {
3737
constructor(private actions$: Actions) {}
38-
**
38+
3939
foo$ = createEffect(() => this.actions$.pipe(
4040
ofType<LoginAction>(AuthActions.login),
4141
tap(action => console.log(action))
@@ -63,7 +63,7 @@ describe('Creator migration', () => {
6363
@Injectable()
6464
export class SomeEffectsClass {
6565
constructor(private actions$: Actions) {}
66-
**
66+
6767
bar$ = createEffect(() => this.actions$.pipe(
6868
ofType(AuthActions.login, AuthActions.logout),
6969
tap(action => console.log(action))
@@ -91,7 +91,7 @@ describe('Creator migration', () => {
9191
@Injectable()
9292
export class SomeEffectsClass {
9393
constructor(private actions$: Actions) {}
94-
**
94+
9595
baz$ = createEffect(() => ({ debounce = 300, scheduler = asyncScheduler } = {}) => this.actions$.pipe(
9696
ofType(login),
9797
tap(action => console.log(action))
@@ -126,14 +126,14 @@ describe('Creator migration', () => {
126126
@Injectable()
127127
export class SomeEffectsClass {
128128
constructor(private actions$: Actions) {}
129-
**
129+
130130
@Log()
131131
login$ = createEffect(() => this.actions$.pipe(
132132
ofType('LOGIN'),
133133
map(() => ({ type: 'LOGGED_IN' }))
134134
));
135+
135136
@Log()
136-
**
137137
logout$ = createEffect(() => this.actions$.pipe(
138138
ofType('LOGOUT'),
139139
map(() => ({ type: 'LOGGED_OUT' }))
@@ -182,7 +182,7 @@ describe('Creator migration', () => {
182182
import { Actions, createEffect, ofType } from '@ngrx/effects';
183183
@Injectable()
184184
export class SomeEffectsClass {
185-
**
185+
186186
logout$ = createEffect(() => this.actions$.pipe(
187187
ofType('LOGOUT'),
188188
map(() => ({ type: 'LOGGED_OUT' }))
@@ -221,7 +221,9 @@ describe('Creator migration', () => {
221221

222222
const actual = tree.readContent(effectPath);
223223

224-
// ** for indentation because empty lines are formatted on auto-save
225-
expect(actual).toBe(expected.replace(/\*\*/g, ' '));
224+
const removeEmptyLines = (value: string) =>
225+
value.replace(/^\s*$(?:\r\n?|\n)/gm, '');
226+
227+
expect(removeEmptyLines(actual)).toBe(removeEmptyLines(expected));
226228
}
227229
});

modules/schematics/src/create-effect-migration/index.ts

Lines changed: 26 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -6,25 +6,12 @@ import {
66
RemoveChange,
77
replaceImport,
88
commitChanges,
9+
visitTSSourceFiles,
910
} from '@ngrx/schematics/schematics-core';
1011

1112
export function migrateToCreators(): Rule {
12-
return (host: Tree) =>
13-
host.visit((path) => {
14-
if (!path.endsWith('.ts')) {
15-
return;
16-
}
17-
18-
const sourceFile = ts.createSourceFile(
19-
path,
20-
host.read(path)!.toString(),
21-
ts.ScriptTarget.Latest
22-
);
23-
24-
if (sourceFile.isDeclarationFile) {
25-
return;
26-
}
27-
13+
return (tree: Tree) => {
14+
visitTSSourceFiles(tree, (sourceFile) => {
2815
const effectsPerClass = sourceFile.statements
2916
.filter(ts.isClassDeclaration)
3017
.map((clas) =>
@@ -42,37 +29,50 @@ export function migrateToCreators(): Rule {
4229
[]
4330
);
4431

45-
const createEffectsChanges = replaceEffectDecorators(host, path, effects);
32+
const createEffectsChanges = replaceEffectDecorators(
33+
tree,
34+
sourceFile,
35+
effects
36+
);
4637
const importChanges = replaceImport(
4738
sourceFile,
48-
path,
39+
sourceFile.fileName as Path,
4940
'@ngrx/effects',
5041
'Effect',
5142
'createEffect'
5243
);
5344

54-
return commitChanges(host, sourceFile.fileName, [
45+
commitChanges(tree, sourceFile.fileName, [
5546
...importChanges,
5647
...createEffectsChanges,
5748
]);
5849
});
50+
};
5951
}
6052

6153
function replaceEffectDecorators(
6254
host: Tree,
63-
path: Path,
55+
sourceFile: ts.SourceFile,
6456
effects: ts.PropertyDeclaration[]
6557
) {
6658
const inserts = effects
6759
.filter((effect) => !!effect.initializer)
6860
.map((effect) => {
6961
const decorator = (effect.decorators || []).find(isEffectDecorator)!;
70-
const effectArguments = getDispatchProperties(host, path, decorator);
62+
const effectArguments = getDispatchProperties(
63+
host,
64+
sourceFile.text,
65+
decorator
66+
);
7167
const end = effectArguments ? `, ${effectArguments})` : ')';
7268

7369
return [
74-
new InsertChange(path, effect.initializer!.pos, ' createEffect(() =>'),
75-
new InsertChange(path, effect.initializer!.end, end),
70+
new InsertChange(
71+
sourceFile.fileName,
72+
effect.initializer!.pos,
73+
' createEffect(() =>'
74+
),
75+
new InsertChange(sourceFile.fileName, effect.initializer!.end, end),
7676
];
7777
})
7878
.reduce((acc, inserts) => acc.concat(inserts), []);
@@ -84,7 +84,7 @@ function replaceEffectDecorators(
8484
const effectDecorators = decorators!.filter(isEffectDecorator);
8585
return effectDecorators.map((decorator) => {
8686
return new RemoveChange(
87-
path,
87+
sourceFile.fileName,
8888
decorator.expression.pos - 1, // also get the @ sign
8989
decorator.expression.end
9090
);
@@ -105,16 +105,15 @@ function isEffectDecorator(decorator: ts.Decorator) {
105105

106106
function getDispatchProperties(
107107
host: Tree,
108-
path: Path,
108+
fileContent: string,
109109
decorator: ts.Decorator
110110
) {
111111
if (!decorator.expression || !ts.isCallExpression(decorator.expression)) {
112112
return '';
113113
}
114114

115115
// just copy the effect properties
116-
const content = host.read(path)!.toString('utf8');
117-
const args = content
116+
const args = fileContent
118117
.substring(
119118
decorator.expression.arguments.pos,
120119
decorator.expression.arguments.end

0 commit comments

Comments
 (0)