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

feat(component): deprecate ReactiveComponentModule #3451

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
5 changes: 5 additions & 0 deletions modules/component/src/reactive-component.module.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,11 @@ import { NgModule } from '@angular/core';
import { LetModule } from './let/let.module';
import { PushModule } from './push/push.module';

/**
* @deprecated `ReactiveComponentModule` is deprecated in favor of `LetModule` and `PushModule`.
* See the {@link https://ngrx.io/guide/migration/v14#reactivecomponentmodule migration guide}
* for more information.
*/
@NgModule({
exports: [LetModule, PushModule],
})
Expand Down
60 changes: 20 additions & 40 deletions modules/schematics/src/ngrx-push-migration/index.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -76,8 +76,8 @@ describe('NgrxPush migration', () => {
});
});

describe('importReactiveComponentModule', () => {
it('should import ReactiveComponentModule when BrowserModule is imported', async () => {
describe('importPushModule', () => {
it('should import PushModule when BrowserModule is imported', async () => {
appTree.create(
'./sut.module.ts',
`
Expand All @@ -100,15 +100,11 @@ describe('NgrxPush migration', () => {
.toPromise();

const actual = tree.readContent('./sut.module.ts');
expect(actual).toMatch(
/imports: \[ BrowserModule, ReactiveComponentModule \],/
);
expect(actual).toMatch(
/import { ReactiveComponentModule } from '@ngrx\/component'/
);
expect(actual).toMatch(/imports: \[ BrowserModule, PushModule \],/);
expect(actual).toMatch(/import { PushModule } from '@ngrx\/component'/);
});

it('should import ReactiveComponentModule when CommonModule is imported', async () => {
it('should import PushModule when CommonModule is imported', async () => {
appTree.create(
'./sut.module.ts',
`
Expand All @@ -131,15 +127,11 @@ describe('NgrxPush migration', () => {
.toPromise();

const actual = tree.readContent('./sut.module.ts');
expect(actual).toMatch(
/imports: \[ CommonModule, ReactiveComponentModule \],/
);
expect(actual).toMatch(
/import { ReactiveComponentModule } from '@ngrx\/component'/
);
expect(actual).toMatch(/imports: \[ CommonModule, PushModule \],/);
expect(actual).toMatch(/import { PushModule } from '@ngrx\/component'/);
});

it("should not import ReactiveComponentModule when it doesn't need to", async () => {
it("should not import PushModule when it doesn't need to", async () => {
appTree.create(
'./sut.module.ts',
`
Expand All @@ -158,17 +150,15 @@ describe('NgrxPush migration', () => {
.toPromise();

const actual = tree.readContent('./sut.module.ts');
expect(actual).not.toMatch(/imports: \[ CommonModule, PushModule \],/);
expect(actual).not.toMatch(
/imports: \[ CommonModule, ReactiveComponentModule \],/
);
expect(actual).not.toMatch(
/import { ReactiveComponentModule } from '@ngrx\/component'/
/import { PushModule } from '@ngrx\/component'/
);
});
});

describe('exportReactiveComponentModule', () => {
it('should export ReactiveComponentModule when BrowserModule is exported', async () => {
describe('exportPushModule', () => {
it('should export PushModule when BrowserModule is exported', async () => {
appTree.create(
'./sut.module.ts',
`
Expand All @@ -191,15 +181,11 @@ describe('NgrxPush migration', () => {
.toPromise();

const actual = tree.readContent('./sut.module.ts');
expect(actual).toMatch(
/exports: \[ BrowserModule, ReactiveComponentModule \],/
);
expect(actual).toMatch(
/import { ReactiveComponentModule } from '@ngrx\/component'/
);
expect(actual).toMatch(/exports: \[ BrowserModule, PushModule \],/);
expect(actual).toMatch(/import { PushModule } from '@ngrx\/component'/);
});

it('should export ReactiveComponentModule when CommonModule is exported', async () => {
it('should export PushModule when CommonModule is exported', async () => {
appTree.create(
'./sut.module.ts',
`
Expand All @@ -222,15 +208,11 @@ describe('NgrxPush migration', () => {
.toPromise();

const actual = tree.readContent('./sut.module.ts');
expect(actual).toMatch(
/exports: \[ CommonModule, ReactiveComponentModule \],/
);
expect(actual).toMatch(
/import { ReactiveComponentModule } from '@ngrx\/component'/
);
expect(actual).toMatch(/exports: \[ CommonModule, PushModule \],/);
expect(actual).toMatch(/import { PushModule } from '@ngrx\/component'/);
});

it("should not export ReactiveComponentModule when it doesn't need to", async () => {
it("should not export PushModule when it doesn't need to", async () => {
appTree.create(
'./sut.module.ts',
`
Expand All @@ -249,11 +231,9 @@ describe('NgrxPush migration', () => {
.toPromise();

const actual = tree.readContent('./sut.module.ts');
expect(actual).not.toMatch(/exports: \[ CommonModule, PushModule \],/);
expect(actual).not.toMatch(
/exports: \[ CommonModule, ReactiveComponentModule \],/
);
expect(actual).not.toMatch(
/import { ReactiveComponentModule } from '@ngrx\/component'/
/import { PushModule } from '@ngrx\/component'/
);
});
});
Expand Down
32 changes: 14 additions & 18 deletions modules/schematics/src/ngrx-push-migration/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,11 @@ import {
} from '../../schematics-core';

const ASYNC_REGEXP = /\| {0,}async/g;
const REACTIVE_MODULE = 'ReactiveComponentModule';
const PUSH_MODULE = 'PushModule';
const COMPONENT_MODULE = '@ngrx/component';

const reactiveModuleToFind = (node: ts.Node) =>
ts.isIdentifier(node) && node.text === REACTIVE_MODULE;
const pushModuleToFind = (node: ts.Node) =>
ts.isIdentifier(node) && node.text === PUSH_MODULE;

const ngModulesToFind = (node: ts.Node) =>
ts.isIdentifier(node) &&
Expand Down Expand Up @@ -46,22 +46,22 @@ export function migrateToNgrxPush(): Rule {
});
}

export function importReactiveComponentModule(): Rule {
export function importPushModule(): Rule {
return (host: Tree) => {
visitTSSourceFiles(host, (sourceFile) => {
let hasCommonModuleOrBrowserModule = false;
let hasReactiveComponentModule = false;
let hasPushModule = false;

visitNgModuleImports(sourceFile, (_, importNodes) => {
hasCommonModuleOrBrowserModule = importNodes.some(ngModulesToFind);
hasReactiveComponentModule = importNodes.some(reactiveModuleToFind);
hasPushModule = importNodes.some(pushModuleToFind);
});

if (hasCommonModuleOrBrowserModule && !hasReactiveComponentModule) {
if (hasCommonModuleOrBrowserModule && !hasPushModule) {
const changes: Change[] = addImportToModule(
sourceFile,
sourceFile.fileName,
REACTIVE_MODULE,
PUSH_MODULE,
COMPONENT_MODULE
);
commitChanges(host, sourceFile.fileName, changes);
Expand All @@ -70,22 +70,22 @@ export function importReactiveComponentModule(): Rule {
};
}

export function exportReactiveComponentModule(): Rule {
export function exportPushModule(): Rule {
return (host: Tree) => {
visitTSSourceFiles(host, (sourceFile) => {
let hasCommonModuleOrBrowserModule = false;
let hasReactiveComponentModule = false;
let hasPushModule = false;

visitNgModuleExports(sourceFile, (_, exportNodes) => {
hasCommonModuleOrBrowserModule = exportNodes.some(ngModulesToFind);
hasReactiveComponentModule = exportNodes.some(reactiveModuleToFind);
hasPushModule = exportNodes.some(pushModuleToFind);
});

if (hasCommonModuleOrBrowserModule && !hasReactiveComponentModule) {
if (hasCommonModuleOrBrowserModule && !hasPushModule) {
const changes: Change[] = addExportToModule(
sourceFile,
sourceFile.fileName,
REACTIVE_MODULE,
PUSH_MODULE,
COMPONENT_MODULE
);
commitChanges(host, sourceFile.fileName, changes);
Expand All @@ -95,9 +95,5 @@ export function exportReactiveComponentModule(): Rule {
}

export default function (): Rule {
return chain([
migrateToNgrxPush(),
importReactiveComponentModule(),
exportReactiveComponentModule(),
]);
return chain([migrateToNgrxPush(), importPushModule(), exportPushModule()]);
}
7 changes: 7 additions & 0 deletions projects/ngrx.io/content/guide/component/let.md
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,13 @@ import { ReactiveComponentModule } from '@ngrx/component';
export class MyFeatureModule {}
```

<div class="alert is-critical">

`ReactiveComponentModule` is deprecated in favor of `LetModule`.
See the [migration guide](guide/migration/v14#reactivecomponentmodule) for more information.

</div>

## Comparison with `*ngIf` and `async`

The current way of binding an observable to the view looks like this:
Expand Down
7 changes: 7 additions & 0 deletions projects/ngrx.io/content/guide/component/push.md
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,13 @@ import { ReactiveComponentModule } from '@ngrx/component';
export class MyFeatureModule {}
```

<div class="alert is-critical">

`ReactiveComponentModule` is deprecated in favor of `PushModule`.
See the [migration guide](guide/migration/v14#reactivecomponentmodule) for more information.

</div>

## Comparison with `async` Pipe

The current way of binding an observable to the view looks like this:
Expand Down
89 changes: 78 additions & 11 deletions projects/ngrx.io/content/guide/migration/v14.md
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ AFTER:
- `e` will be thrown error when `obs$` emits error event.
- `e` will be `undefined` when `obs$` emits next/complete event.

#### add ability to pass non-observable values
#### Add ability to pass non-observable values

1. The context of `LetDirective` is strongly typed when `null` or
`undefined` is passed as input.
Expand Down Expand Up @@ -105,27 +105,27 @@ BEFORE:

```ts
@Component({
template: `
<p *ngrxLet="numbers as n">{{ n }}</p>
<p>{{ numbers | ngrxPush }}</p>
`,
template: `
<p *ngrxLet="numbers as n">{{ n }}</p>
<p>{{ numbers | ngrxPush }}</p>
`,
})
export class NumbersComponent {
numbers = [1, 2, 3];
numbers = [1, 2, 3];
}
```

AFTER:

```ts
@Component({
template: `
<p *ngrxLet="numbers$ as n">{{ n }}</p>
<p>{{ numbers$ | ngrxPush }}</p>
`,
template: `
<p *ngrxLet="numbers$ as n">{{ n }}</p>
<p>{{ numbers$ | ngrxPush }}</p>
`,
})
export class NumbersComponent {
numbers$ = from([1, 2, 3]);
numbers$ = from([1, 2, 3]);
}
```

Expand All @@ -139,3 +139,70 @@ When the schematics are installed, `@ngrx/schematics` is added to the `schematic
<div class="alert is-helpful">
A migration is provided to add `@ngrx/schematics` to the `schematicCollections`.
</div>

## Deprecations

### @ngrx/component

#### ReactiveComponentModule

`ReactiveComponentModule` is deprecated in favor of `LetModule` and `PushModule`.

BEFORE:

```ts
import { ReactiveComponentModule } from '@ngrx/component';

@NgModule({
imports: [
// ... other imports
ReactiveComponentModule,
],
})
export class MyFeatureModule {}
```

AFTER:

If the components declared in the `MyFeatureModule` use only the `*ngrxLet` directive:

```ts
import { LetModule } from '@ngrx/component';

@NgModule({
imports: [
// ... other imports
LetModule,
],
})
export class MyFeatureModule {}
```

If the components declared in the `MyFeatureModule` use only the `ngrxPush` pipe:

```ts
import { PushModule } from '@ngrx/component';

@NgModule({
imports: [
// ... other imports
PushModule,
],
})
export class MyFeatureModule {}
```

If the components declared in the `MyFeatureModule` use both the `*ngrxLet` directive and the `ngrxPush` pipe:

```ts
import { LetModule, PushModule } from '@ngrx/component';

@NgModule({
imports: [
// ... other imports
LetModule,
PushModule,
],
})
export class MyFeatureModule {}
```