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

Author: rainerhahnekamp

modules/effects/migrations/18_0_0-beta/index.spec.ts

@@ -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');
@@ -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')
     );

modules/effects/migrations/18_0_0-beta/index.ts

@@ -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;
       }
@@ -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(
@@ -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
           )
         );
       }