Skip to content

Commit

Permalink
feat(effects): move createEffect migration to ng-update migration (#3074
Browse files Browse the repository at this point in the history 
)

BREAKING CHANGE:

The create-effect-migration migration is removed 

BEFORE:

The Effect decorator removal and migration are done manually through schematics.

AFTER:

The Effect decorator removal and migration are performed automatically on upgrade to version 13 of NgRx Effects.
  • Loading branch information
@timdeschryver
timdeschryver authored Nov 1, 2021
1 parent 8cc930b commit 5974913
Show file tree
Hide file tree
Showing 7 changed files with 100 additions and 81 deletions.
153 changes: 91 additions & 62 deletions ...src/create-effect-migration/index.spec.ts → ...s/effects/migrations/13_0_0/index.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,20 +6,22 @@ import {
import * as path from 'path';
import { createWorkspace } from '@ngrx/schematics-core/testing';

describe('Creator migration', () => {
const schematicRunner = new SchematicTestRunner(
'@ngrx/schematics',
path.join(__dirname, '../../collection.json')
);

let appTree: UnitTestTree;

beforeEach(async () => {
appTree = await createWorkspace(schematicRunner, appTree);
});

it('should use createEffect for non-dispatching effects', async () => {
const input = tags.stripIndent`
describe('Effects Migration 13_0_0', () => {
describe('@Effect to createEffect', () => {
const collectionPath = path.join(__dirname, '../migration.json');
const schematicRunner = new SchematicTestRunner(
'schematics',
collectionPath
);

let appTree: UnitTestTree;

beforeEach(async () => {
appTree = await createWorkspace(schematicRunner, appTree);
});

it('migrates to createEffect for dispatching effects', async () => {
const input = tags.stripIndent`
@Injectable()
export class SomeEffectsClass {
constructor(private actions$: Actions) {}
Expand All @@ -31,7 +33,7 @@ describe('Creator migration', () => {
}
`;

const output = tags.stripIndent`
const output = tags.stripIndent`
@Injectable()
export class SomeEffectsClass {
constructor(private actions$: Actions) {}
Expand All @@ -43,11 +45,11 @@ describe('Creator migration', () => {
}
`;

await runTest(input, output);
});
await runTest(input, output);
});

it('should use createEffect for non-dispatching effects', async () => {
const input = tags.stripIndent`
it('migrates to createEffect for non-dispatching effects', async () => {
const input = tags.stripIndent`
@Injectable()
export class SomeEffectsClass {
constructor(private actions$: Actions) {}
Expand All @@ -59,7 +61,7 @@ describe('Creator migration', () => {
}
`;

const output = tags.stripIndent`
const output = tags.stripIndent`
@Injectable()
export class SomeEffectsClass {
constructor(private actions$: Actions) {}
Expand All @@ -71,11 +73,11 @@ describe('Creator migration', () => {
}
`;

await runTest(input, output);
});
await runTest(input, output);
});

it('should use createEffect for effects as functions', async () => {
const input = tags.stripIndent`
it('migrates to createEffect for effects as functions', async () => {
const input = tags.stripIndent`
@Injectable()
export class SomeEffectsClass {
constructor(private actions$: Actions) {}
Expand All @@ -87,7 +89,7 @@ describe('Creator migration', () => {
}
`;

const output = tags.stripIndent`
const output = tags.stripIndent`
@Injectable()
export class SomeEffectsClass {
constructor(private actions$: Actions) {}
Expand All @@ -99,11 +101,11 @@ describe('Creator migration', () => {
}
`;

await runTest(input, output);
});
await runTest(input, output);
});

it('should stay off of other decorators', async () => {
const input = tags.stripIndent`
it('keeps other decorators', async () => {
const input = tags.stripIndent`
@Injectable()
export class SomeEffectsClass {
constructor(private actions$: Actions) {}
Expand All @@ -122,7 +124,7 @@ describe('Creator migration', () => {
}
`;

const output = tags.stripIndent`
const output = tags.stripIndent`
@Injectable()
export class SomeEffectsClass {
constructor(private actions$: Actions) {}
Expand All @@ -141,31 +143,31 @@ describe('Creator migration', () => {
}
`;

await runTest(input, output);
});
await runTest(input, output);
});

it('should import createEffect', async () => {
const input = tags.stripIndent`
it('imports createEffect from effects', async () => {
const input = tags.stripIndent`
import { Actions, ofType, Effect } from '@ngrx/effects';
@Injectable()
export class SomeEffectsClass {
constructor(private actions$: Actions) {}
}
`;

const output = tags.stripIndent`
const output = tags.stripIndent`
import { Actions, ofType, createEffect } from '@ngrx/effects';
@Injectable()
export class SomeEffectsClass {
constructor(private actions$: Actions) {}
}
`;

await runTest(input, output);
});
await runTest(input, output);
});

it('should not import createEffect if already imported', async () => {
const input = tags.stripIndent`
it('does not import createEffect if already imported', async () => {
const input = tags.stripIndent`
import { Actions, Effect, createEffect, ofType } from '@ngrx/effects';
@Injectable()
export class SomeEffectsClass {
Expand All @@ -178,7 +180,7 @@ describe('Creator migration', () => {
}
`;

const output = tags.stripIndent`
const output = tags.stripIndent`
import { Actions, createEffect, ofType } from '@ngrx/effects';
@Injectable()
export class SomeEffectsClass {
Expand All @@ -191,11 +193,11 @@ describe('Creator migration', () => {
}
`;

await runTest(input, output);
});
await runTest(input, output);
});

it('should not run the schematic if the createEffect syntax is already used', async () => {
const input = tags.stripIndent`
it('does not migrate if the createEffect syntax is already used', async () => {
const input = tags.stripIndent`
import { Actions, createEffect, ofType } from '@ngrx/effects';
@Injectable()
export class SomeEffectsClass {
Expand All @@ -207,23 +209,50 @@ describe('Creator migration', () => {
}
`;

await runTest(input, input);
await runTest(input, input);
});

it('removes the @Effect decorator', async () => {
const input = tags.stripIndent`
import { Actions, createEffect, ofType } from '@ngrx/effects';
@Injectable()
export class SomeEffectsClass {
@Effect()
logout$ = createEffect(() => this.actions$.pipe(
ofType('LOGOUT'),
map(() => ({ type: 'LOGGED_OUT' }))
));
constructor(private actions$: Actions) {}
}
`;
const output = tags.stripIndent`
import { Actions, createEffect, ofType } from '@ngrx/effects';
@Injectable()
export class SomeEffectsClass {
logout$ = createEffect(() => this.actions$.pipe(
ofType('LOGOUT'),
map(() => ({ type: 'LOGGED_OUT' }))
));
constructor(private actions$: Actions) {}
}
`;
await runTest(input, output);
});

async function runTest(input: string, expected: string) {
const effectPath = '/some.effects.ts';
appTree.create(effectPath, input);

const tree = await schematicRunner
.runSchematicAsync(`ngrx-effects-migration-03`, {}, appTree)
.toPromise();

const actual = tree.readContent(effectPath);

const removeEmptyLines = (value: string) =>
value.replace(/^\s*$(?:\r\n?|\n)/gm, '');

expect(removeEmptyLines(actual)).toBe(removeEmptyLines(expected));
}
});

async function runTest(input: string, expected: string) {
const options = {};
const effectPath = '/some.effects.ts';
appTree.create(effectPath, input);

const tree = await schematicRunner
.runSchematicAsync('create-effect-migration', options, appTree)
.toPromise();

const actual = tree.readContent(effectPath);

const removeEmptyLines = (value: string) =>
value.replace(/^\s*$(?:\r\n?|\n)/gm, '');

expect(removeEmptyLines(actual)).toBe(removeEmptyLines(expected));
}
});
4 changes: 3 additions & 1 deletion ...tics/src/create-effect-migration/index.ts → modules/effects/migrations/13_0_0/index.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,6 @@ function replaceEffectDecorators(
effects: ts.PropertyDeclaration[]
) {
const inserts = effects
.filter((effect) => !!effect.initializer)
.map((effect) => {
if (!effect.initializer) {
return [];
Expand All @@ -65,6 +64,9 @@ function replaceEffectDecorators(
if (!decorator) {
return [];
}
if (effect.initializer.getText().includes('createEffect')) {
return [];
}
const effectArguments = getDispatchProperties(
host,
sourceFile.text,
Expand Down
5 changes: 5 additions & 0 deletions modules/effects/migrations/migration.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,11 @@
"description": "The road to v9",
"version": "9-beta",
"factory": "./9_0_0/index"
},
"ngrx-effects-migration-03": {
"description": "The road to v13",
"version": "13",
"factory": "./13_0_0/index"
}
}
}
7 changes: 0 additions & 7 deletions modules/schematics/collection.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,13 +22,6 @@
"description": "Add side effect class"
},

"create-effect-migration": {
"aliases": ["cefm"],
"factory": "./src/create-effect-migration",
"schema": "./src/create-effect-migration/schema.json",
"description": "Migrated usages of @Effect() to createEffect()"
},

"entity": {
"aliases": ["en"],
"factory": "./src/entity",
Expand Down
8 changes: 0 additions & 8 deletions modules/schematics/src/create-effect-migration/schema.json
View file

This file was deleted.

2 changes: 0 additions & 2 deletions modules/schematics/src/create-effect-migration/schema.ts
View file

This file was deleted.

2 changes: 1 addition & 1 deletion projects/ngrx.io/content/guide/migration/v11.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,7 @@ login$ = createEffect(() => {
});
```

To automatically migrate `@Effect` usages to the `createEffect` method, run the following NgRx migration:
To automatically migrate `@Effect` usages to the `createEffect` method, run the following NgRx migration (this migration is only available in v11 and v12):

```sh
ng generate @ngrx/schematics:create-effect-migration
Expand Down

0 comments on commit 5974913

Please sign in to comment.