Skip to content

Commit 931adb1

Browse files
MikeRyanDevbrandonroberts
authored andcommitted
fix(Effects): Start child effects after running root effects (#43)
This removes the app initialization logic altogether. The 'OnRunEffects' lifecycle hook should be sufficient to control when your effects start.
1 parent 0be1bee commit 931adb1

File tree

9 files changed

+96
-84
lines changed

9 files changed

+96
-84
lines changed
Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,58 @@
1+
import { Observable } from 'rxjs/Observable';
2+
import { Notification } from 'rxjs/Notification';
3+
import { Action } from '@ngrx/store';
4+
import { ErrorReporter } from './error_reporter';
5+
6+
export interface EffectNotification {
7+
effect: Observable<any> | (() => Observable<any>);
8+
propertyName: string;
9+
sourceName: string;
10+
sourceInstance: any;
11+
notification: Notification<Action | null | undefined>;
12+
}
13+
14+
export function verifyOutput(output: EffectNotification, reporter: ErrorReporter) {
15+
reportErrorThrown(output, reporter);
16+
reportInvalidActions(output, reporter);
17+
}
18+
19+
function reportErrorThrown(output: EffectNotification, reporter: ErrorReporter) {
20+
if (output.notification.kind === 'E') {
21+
const errorReason = `Effect ${getEffectName(output)} threw an error`;
22+
23+
reporter.report(errorReason, {
24+
Source: output.sourceInstance,
25+
Effect: output.effect,
26+
Error: output.notification.error,
27+
Notification: output.notification,
28+
});
29+
}
30+
}
31+
32+
function reportInvalidActions(output: EffectNotification, reporter: ErrorReporter) {
33+
if (output.notification.kind === 'N') {
34+
const action = output.notification.value;
35+
const isInvalidAction = !isAction(action);
36+
37+
if (isInvalidAction) {
38+
const errorReason = `Effect ${getEffectName(output)} dispatched an invalid action`;
39+
40+
reporter.report(errorReason, {
41+
Source: output.sourceInstance,
42+
Effect: output.effect,
43+
Dispatched: action,
44+
Notification: output.notification,
45+
});
46+
}
47+
}
48+
}
49+
50+
function isAction(action: any): action is Action {
51+
return action && action.type && typeof action.type === 'string';
52+
}
53+
54+
function getEffectName({ propertyName, sourceInstance, sourceName }: EffectNotification) {
55+
const isMethod = typeof sourceInstance[propertyName] === 'function';
56+
57+
return `"${sourceName}.${propertyName}${isMethod ? '()' : ''}"`;
58+
}

modules/effects/src/effect_sources.ts

Lines changed: 8 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -3,17 +3,21 @@ import { mergeMap } from 'rxjs/operator/mergeMap';
33
import { exhaustMap } from 'rxjs/operator/exhaustMap';
44
import { map } from 'rxjs/operator/map';
55
import { dematerialize } from 'rxjs/operator/dematerialize';
6+
import { concat } from 'rxjs/observable/concat';
67
import { Observable } from 'rxjs/Observable';
78
import { Subject } from 'rxjs/Subject';
8-
import { Injectable, isDevMode } from '@angular/core';
9+
import { Injectable } from '@angular/core';
910
import { Action } from '@ngrx/store';
11+
import { EffectNotification, verifyOutput } from './effect_notification';
1012
import { getSourceForInstance } from './effects_metadata';
11-
import { resolveEffectSource, EffectNotification } from './effects_resolver';
13+
import { resolveEffectSource } from './effects_resolver';
1214
import { ErrorReporter } from './error_reporter';
1315

1416
@Injectable()
1517
export class EffectSources extends Subject<any> {
16-
constructor(private errorReporter: ErrorReporter) {
18+
constructor(
19+
private errorReporter: ErrorReporter,
20+
) {
1721
super();
1822
}
1923

@@ -32,38 +36,7 @@ export class EffectSources extends Subject<any> {
3236
map.call(
3337
exhaustMap.call(source$, resolveEffectSource),
3438
(output: EffectNotification) => {
35-
switch (output.notification.kind) {
36-
case 'N': {
37-
const action = output.notification.value;
38-
const isInvalidAction =
39-
!action || !action.type || typeof action.type !== 'string';
40-
41-
if (isInvalidAction) {
42-
const errorReason = `Effect "${output.sourceName}.${output.propertyName}" dispatched an invalid action`;
43-
44-
this.errorReporter.report(errorReason, {
45-
Source: output.sourceInstance,
46-
Effect: output.effect,
47-
Dispatched: action,
48-
Notification: output.notification,
49-
});
50-
}
51-
52-
break;
53-
}
54-
case 'E': {
55-
const errorReason = `Effect "${output.sourceName}.${output.propertyName}" threw an error`;
56-
57-
this.errorReporter.report(errorReason, {
58-
Source: output.sourceInstance,
59-
Effect: output.effect,
60-
Error: output.notification.error,
61-
Notification: output.notification,
62-
});
63-
64-
break;
65-
}
66-
}
39+
verifyOutput(output, this.errorReporter);
6740

6841
return output.notification;
6942
},

modules/effects/src/effects_feature_module.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,16 @@
11
import { NgModule, Inject, Type } from '@angular/core';
2-
import { EffectSources } from './effect_sources';
2+
import { EffectsRootModule } from './effects_root_module';
33
import { FEATURE_EFFECTS } from './tokens';
44

55
@NgModule({})
66
export class EffectsFeatureModule {
77
constructor(
8-
private effectSources: EffectSources,
8+
private root: EffectsRootModule,
99
@Inject(FEATURE_EFFECTS) effectSourceGroups: any[][],
1010
) {
1111
effectSourceGroups.forEach(group =>
1212
group.forEach(effectSourceInstance =>
13-
effectSources.addEffects(effectSourceInstance),
13+
root.addEffects(effectSourceInstance),
1414
),
1515
);
1616
}

modules/effects/src/effects_module.ts

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,9 +3,9 @@ import { EffectSources } from './effect_sources';
33
import { Actions } from './actions';
44
import { ROOT_EFFECTS, FEATURE_EFFECTS, CONSOLE } from './tokens';
55
import { EffectsFeatureModule } from './effects_feature_module';
6+
import { EffectsRootModule } from './effects_root_module';
67
import { EffectsRunner } from './effects_runner';
78
import { ErrorReporter } from './error_reporter';
8-
import { RUN_EFFECTS } from './run_effects';
99

1010
@NgModule({})
1111
export class EffectsModule {
@@ -26,13 +26,12 @@ export class EffectsModule {
2626

2727
static forRoot(rootEffects: Type<any>[]): ModuleWithProviders {
2828
return {
29-
ngModule: EffectsModule,
29+
ngModule: EffectsRootModule,
3030
providers: [
3131
EffectsRunner,
3232
EffectSources,
3333
ErrorReporter,
3434
Actions,
35-
RUN_EFFECTS,
3635
rootEffects,
3736
{
3837
provide: ROOT_EFFECTS,

modules/effects/src/effects_resolver.ts

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -5,17 +5,10 @@ import { map } from 'rxjs/operator/map';
55
import { Observable } from 'rxjs/Observable';
66
import { Notification } from 'rxjs/Notification';
77
import { Action } from '@ngrx/store';
8+
import { EffectNotification } from './effect_notification';
89
import { getSourceMetadata, getSourceForInstance } from './effects_metadata';
910
import { isOnRunEffects } from './on_run_effects';
1011

11-
export interface EffectNotification {
12-
effect: Observable<any> | (() => Observable<any>);
13-
propertyName: string;
14-
sourceName: string;
15-
sourceInstance: any;
16-
notification: Notification<Action | null | undefined>;
17-
}
18-
1912
export function mergeEffects(
2013
sourceInstance: any,
2114
): Observable<EffectNotification> {
Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
import { NgModule, Inject } from '@angular/core';
2+
import { EffectsRunner } from './effects_runner';
3+
import { EffectSources } from './effect_sources';
4+
import { ROOT_EFFECTS } from './tokens';
5+
6+
@NgModule({})
7+
export class EffectsRootModule {
8+
constructor(
9+
private sources: EffectSources,
10+
runner: EffectsRunner,
11+
@Inject(ROOT_EFFECTS) rootEffects: any[],
12+
) {
13+
runner.start();
14+
15+
rootEffects.forEach(effectSourceInstance =>
16+
sources.addEffects(effectSourceInstance)
17+
);
18+
}
19+
20+
addEffects(effectSourceInstance: any) {
21+
this.sources.addEffects(effectSourceInstance);
22+
}
23+
}

modules/effects/src/on_run_effects.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import { Observable } from 'rxjs/Observable';
22
import { getSourceForInstance } from './effects_metadata';
3-
import { EffectNotification } from './effects_resolver';
3+
import { EffectNotification } from './effect_notification';
44

55
export interface OnRunEffects {
66
ngrxOnRunEffects(

modules/effects/src/run_effects.ts

Lines changed: 0 additions & 31 deletions
This file was deleted.

modules/effects/src/tokens.ts

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,9 +3,6 @@ import { InjectionToken, Type } from '@angular/core';
33
export const IMMEDIATE_EFFECTS = new InjectionToken<any[]>(
44
'ngrx/effects: Immediate Effects',
55
);
6-
export const BOOTSTRAP_EFFECTS = new InjectionToken<any[]>(
7-
'ngrx/effects: Bootstrap Effects',
8-
);
96
export const ROOT_EFFECTS = new InjectionToken<Type<any>[]>(
107
'ngrx/effects: Root Effects',
118
);

0 commit comments

Comments
 (0)