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(component-store): resolve issues in migration script to v18 #4403

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
95 changes: 95 additions & 0 deletions modules/component-store/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('ComponentStore Migration to 18.0.0-beta', () => {
const collectionPath = path.join(__dirname, '../migration.json');
Expand Down Expand Up @@ -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')
Expand Down
29 changes: 23 additions & 6 deletions modules/component-store/migrations/18_0_0-beta/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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(
Expand Down Expand Up @@ -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
)
);
}
Expand Down