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.

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

fix(effects): fix bugs in migration script to v18 #4402

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
91 changes: 90 additions & 1 deletion modules/effects/migrations/18_0_0-beta/index.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import {
import { createWorkspace } from '@ngrx/schematics-core/testing';
import { tags } from '@angular-devkit/core';
import * as path from 'path';
import { LogEntry } from '@angular-devkit/core/src/logger';

describe('Effects Migration to 18.0.0-beta', () => {
const collectionPath = path.join(__dirname, '../migration.json');
Expand Down Expand Up @@ -121,7 +122,95 @@ export class SomeEffects {
});
});

it('should add if they are missing', async () => {
it('should work with prior import from same namespace', async () => {
const input = tags.stripIndent`
import { Actions } from '@ngrx/effects';
import { concatLatestFrom, createEffect, ofType } from '@ngrx/effects';

class SomeEffects {}
`;
const output = tags.stripIndent`
import { Actions } from '@ngrx/effects';
import { createEffect, ofType } from '@ngrx/effects';
import { concatLatestFrom } from '@ngrx/operators';

class SomeEffects {}
`;
await verifySchematic(input, output);
});

it('should operate on multiple files', async () => {
const inputMainOne = tags.stripIndent`
import { Actions, concatLatestFrom, createEffect, ofType } from '@ngrx/effects';
import { tap } from 'rxjs/operators';

class SomeEffects {}
`;

const outputMainOne = tags.stripIndent`
import { Actions, createEffect, ofType } from '@ngrx/effects';
import { concatLatestFrom } from '@ngrx/operators';
import { tap } from 'rxjs/operators';

class SomeEffects {}
`;

const inputMainTwo = tags.stripIndent`
import { provideEffects } from '@ngrx/effects';
import { Actions, concatLatestFrom, createEffect, ofType } from '@ngrx/effects';

class SomeEffects {}
`;

const outputMainTwo = tags.stripIndent`
import { provideEffects } from '@ngrx/effects';
import { Actions, createEffect, ofType } from '@ngrx/effects';
import { concatLatestFrom } from '@ngrx/operators';

class SomeEffects {}
`;
appTree.create('mainOne.ts', inputMainOne);
appTree.create('mainTwo.ts', inputMainTwo);

const tree = await schematicRunner.runSchematic(
`ngrx-effects-migration-18-beta`,
{},
appTree
);

const actualMainOne = tree.readContent('mainOne.ts');
const actualMainTwo = tree.readContent('mainTwo.ts');

expect(actualMainOne).toBe(outputMainOne);
expect(actualMainTwo).toBe(outputMainTwo);
});

it('should report an info on multiple imports of concatLatestFrom', async () => {
const input = tags.stripIndent`
import { concatLatestFrom } from '@ngrx/effects';
import { concatLatestFrom, createEffect, ofType } from '@ngrx/effects';

class SomeEffects {}
`;

appTree.create('main.ts', input);
const logEntries: LogEntry[] = [];
schematicRunner.logger.subscribe((logEntry) => logEntries.push(logEntry));
await schematicRunner.runSchematic(
`ngrx-effects-migration-18-beta`,
{},
appTree
);

expect(logEntries).toHaveLength(1);
expect(logEntries[0]).toMatchObject({
message:
'[@ngrx/effects] Skipping because of multiple `concatLatestFrom` imports',
level: 'info',
});
});

it('should add @ngrx/operators if they are missing', async () => {
const originalPackageJson = JSON.parse(
appTree.readContent('/package.json')
);
Expand Down
28 changes: 23 additions & 5 deletions modules/effects/migrations/18_0_0-beta/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,26 +17,43 @@ import { createRemoveChange } from '../../schematics-core/utility/change';

export function migrateConcatLatestFromImport(): Rule {
return (tree: Tree, ctx: SchematicContext) => {
const changes: Change[] = [];
addPackageToPackageJson(tree, 'dependencies', '@ngrx/operators', '^18.0.0');

visitTSSourceFiles(tree, (sourceFile) => {
const importDeclarations = new Array<ts.ImportDeclaration>();

getImportDeclarations(sourceFile, importDeclarations);

const effectsImportsAndDeclaration = importDeclarations
const effectsImportsAndDeclarations = importDeclarations
.map((effectsImportDeclaration) => {
const effectsImports = getEffectsNamedBinding(
effectsImportDeclaration
);
if (effectsImports) {
return { effectsImports, effectsImportDeclaration };
if (
effectsImports.elements.some(
(element) => element.name.getText() === 'concatLatestFrom'
)
) {
return { effectsImports, effectsImportDeclaration };
}
return undefined;
} else {
return undefined;
}
})
.find(Boolean);
.filter(Boolean);

if (effectsImportsAndDeclarations.length === 0) {
return;
} else if (effectsImportsAndDeclarations.length > 1) {
ctx.logger.info(
'[@ngrx/effects] Skipping because of multiple `concatLatestFrom` imports'
);
return;
}

const [effectsImportsAndDeclaration] = effectsImportsAndDeclarations;
if (!effectsImportsAndDeclaration) {
return;
}
Expand All @@ -53,6 +70,7 @@ export function migrateConcatLatestFromImport(): Rule {
.map((element) => element.name.getText())
.join(', ');

const changes: Change[] = [];
// Remove `concatLatestFrom` from @ngrx/effects and leave the other imports
if (otherEffectsImports) {
changes.push(
Expand Down Expand Up @@ -107,7 +125,7 @@ export function migrateConcatLatestFromImport(): Rule {
new InsertChange(
sourceFile.fileName,
effectsImportDeclaration.getEnd() + 1,
`${newOperatorsImport}\n`
`${newOperatorsImport}\n` // not os-independent for snapshot tests
)
);
}
Expand Down