fix(component-store): resolve issues in migration script to v18 (#4403)

Author: rainerhahnekamp

modules/component-store/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('ComponentStore Migration to 18.0.0-beta', () => {
   const collectionPath = path.join(__dirname, '../migration.json');
@@ -117,6 +118,100 @@ export class MyStore extends ComponentStore {
     });
   });
 
+  it('should work with prior import from same namespace', async () => {
+    const input = tags.stripIndent`
+import { ComponentStore, provideComponentStore } from '@ngrx/component-store';
+import { tapResponse } from '@ngrx/component-store';
+
+export class MyStore extends ComponentStore {}
+      `;
+    const output = tags.stripIndent`
+import { ComponentStore, provideComponentStore } from '@ngrx/component-store';
+import { tapResponse } from '@ngrx/operators';
+
+export class MyStore extends ComponentStore {}
+      `;
+    await verifySchematic(input, output);
+  });
+
+  it('should operate on multiple files', async () => {
+    const inputMainOne = tags.stripIndent`
+import { ComponentStore, tapResponse } from '@ngrx/component-store';
+import { concatLatestFrom } from '@ngrx/operators';
+
+@Injectable()
+export class MyStore extends ComponentStore {
+
+}
+`;
+
+    const outputMainOne = tags.stripIndent`
+import { ComponentStore } from '@ngrx/component-store';
+import { concatLatestFrom, tapResponse } from '@ngrx/operators';
+
+@Injectable()
+export class MyStore extends ComponentStore {
+
+}
+`;
+
+    const inputMainTwo = tags.stripIndent`
+import { tapResponse } from "@ngrx/component-store";
+
+@Injectable()
+export class MyStore extends ComponentStore {
+
+}
+      `;
+    const outputMainTwo = tags.stripIndent`
+import { tapResponse } from '@ngrx/operators';
+
+@Injectable()
+export class MyStore extends ComponentStore {
+
+}
+`;
+    appTree.create('mainOne.ts', inputMainOne);
+    appTree.create('mainTwo.ts', inputMainTwo);
+
+    const tree = await schematicRunner.runSchematic(
+      `ngrx-component-store-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 a warning on multiple imports of tapResponse', async () => {
+    const input = tags.stripIndent`
+import { tapResponse } from '@ngrx/component-store';
+import { tapResponse, ComponentStore } from '@ngrx/component-store';
+
+class SomeEffects {}
+      `;
+
+    appTree.create('main.ts', input);
+    const logEntries: LogEntry[] = [];
+    schematicRunner.logger.subscribe((logEntry) => logEntries.push(logEntry));
+    await schematicRunner.runSchematic(
+      `ngrx-component-store-migration-18-beta`,
+      {},
+      appTree
+    );
+
+    expect(logEntries).toHaveLength(1);
+    expect(logEntries[0]).toMatchObject({
+      message:
+        '[@ngrx/component-store] Skipping because of multiple `tapResponse` imports',
+      level: 'info',
+    });
+  });
+
   it('should add @ngrx/operators if they are missing', async () => {
     const originalPackageJson = JSON.parse(
       appTree.readContent('/package.json')

modules/component-store/migrations/18_0_0-beta/index.ts

@@ -17,30 +17,46 @@ import { createRemoveChange } from '../../schematics-core/utility/change';
 
 export function migrateTapResponseImport(): 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 componentStoreImportsAndDeclaration = importDeclarations
+      const componentStoreImportsAndDeclarations = importDeclarations
         .map((componentStoreImportDeclaration) => {
           const componentStoreImports = getComponentStoreNamedBinding(
             componentStoreImportDeclaration
           );
           if (componentStoreImports) {
-            return { componentStoreImports, componentStoreImportDeclaration };
+            if (
+              componentStoreImports.elements.some(
+                (element) => element.name.getText() === 'tapResponse'
+              )
+            ) {
+              return { componentStoreImports, componentStoreImportDeclaration };
+            }
+            return undefined;
           } else {
             return undefined;
           }
         })
-        .find(Boolean);
+        .filter(Boolean);
 
-      if (!componentStoreImportsAndDeclaration) {
+      if (componentStoreImportsAndDeclarations.length === 0) {
+        return;
+      } else if (componentStoreImportsAndDeclarations.length > 1) {
+        ctx.logger.info(
+          '[@ngrx/component-store] Skipping because of multiple `tapResponse` imports'
+        );
         return;
       }
 
+      const [componentStoreImportsAndDeclaration] =
+        componentStoreImportsAndDeclarations;
+      if (!componentStoreImportsAndDeclaration) {
+        return;
+      }
       const { componentStoreImports, componentStoreImportDeclaration } =
         componentStoreImportsAndDeclaration;
 
@@ -53,6 +69,7 @@ export function migrateTapResponseImport(): Rule {
         .map((element) => element.name.getText())
         .join(', ');
 
+      const changes: Change[] = [];
       // Remove `tapResponse` from @ngrx/component-store and leave the other imports
       if (otherComponentStoreImports) {
         changes.push(
@@ -107,7 +124,7 @@ export function migrateTapResponseImport(): Rule {
           new InsertChange(
             sourceFile.fileName,
             componentStoreImportDeclaration.getEnd() + 1,
-            `${newOperatorsImport}\n`
+            `${newOperatorsImport}\n` // not os-independent for snapshot tests
           )
         );
       }