fix(effects): dispatch init action once (#2164)

Closes #2106 

BREAKING CHANGE:

BEFORE:

When the effect class was registered, the init action would be dispatched.
If the effect was provided in multiple lazy loaded modules, the init action would be dispatched for every module.

AFTER:

The init action is only dispatched once
The init action is now dispatched based on the identifier of the effect (via ngrxOnIdentifyEffects)

Author: timdeschryver

modules/effects/spec/effect_sources.spec.ts

@@ -1,17 +1,28 @@
 import { ErrorHandler } from '@angular/core';
 import { TestBed } from '@angular/core/testing';
 import { cold, hot, getTestScheduler } from 'jasmine-marbles';
-import { concat, NEVER, Observable, of, throwError, timer } from 'rxjs';
-import { concatMap, map } from 'rxjs/operators';
+import {
+  concat,
+  NEVER,
+  Observable,
+  of,
+  throwError,
+  timer,
+  Subject,
+} from 'rxjs';
+import { map, mapTo } from 'rxjs/operators';
 
 import {
   Effect,
   EffectSources,
   OnIdentifyEffects,
   OnInitEffects,
   createEffect,
+  Actions,
 } from '../';
+import { EffectsRunner } from '../src/effects_runner';
 import { Store } from '@ngrx/store';
+import { ofType } from '../src';
 
 describe('EffectSources', () => {
   let mockErrorReporter: ErrorHandler;
@@ -21,6 +32,7 @@ describe('EffectSources', () => {
     TestBed.configureTestingModule({
       providers: [
         EffectSources,
+        EffectsRunner,
         {
           provide: Store,
           useValue: {
@@ -30,6 +42,9 @@ describe('EffectSources', () => {
       ],
     });
 
+    const effectsRunner = TestBed.get(EffectsRunner);
+    effectsRunner.start();
+
     mockErrorReporter = TestBed.get(ErrorHandler);
     effectSources = TestBed.get(EffectSources);
 
@@ -51,15 +66,83 @@ describe('EffectSources', () => {
         return { type: '[EffectWithInitAction] Init' };
       }
     }
+    const store = TestBed.get(Store);
 
     effectSources.addEffects(new EffectWithInitAction());
 
+    expect(store.dispatch).toHaveBeenCalledTimes(1);
+    expect(store.dispatch).toHaveBeenCalledWith({
+      type: '[EffectWithInitAction] Init',
+    });
+  });
+
+  it('should dispatch an action on ngrxOnInitEffects after being registered (class has effects)', () => {
+    class EffectWithInitActionAndEffects implements OnInitEffects {
+      effectOne = createEffect(() => {
+        return this.actions$.pipe(
+          ofType('Action 1'),
+          mapTo({ type: 'Action 1 Response' })
+        );
+      });
+      effectTwo = createEffect(() => {
+        return this.actions$.pipe(
+          ofType('Action 2'),
+          mapTo({ type: 'Action 2 Response' })
+        );
+      });
+
+      ngrxOnInitEffects() {
+        return { type: '[EffectWithInitAction] Init' };
+      }
+
+      constructor(private actions$: Actions) {}
+    }
     const store = TestBed.get(Store);
+
+    effectSources.addEffects(new EffectWithInitActionAndEffects(new Subject()));
+
+    expect(store.dispatch).toHaveBeenCalledTimes(1);
     expect(store.dispatch).toHaveBeenCalledWith({
       type: '[EffectWithInitAction] Init',
     });
   });
 
+  it('should only dispatch an action on ngrxOnInitEffects once after being registered', () => {
+    class EffectWithInitAction implements OnInitEffects {
+      ngrxOnInitEffects() {
+        return { type: '[EffectWithInitAction] Init' };
+      }
+    }
+    const store = TestBed.get(Store);
+
+    effectSources.addEffects(new EffectWithInitAction());
+    effectSources.addEffects(new EffectWithInitAction());
+
+    expect(store.dispatch).toHaveBeenCalledTimes(1);
+  });
+
+  it('should dispatch an action on ngrxOnInitEffects multiple times after being registered with different identifiers', () => {
+    let id = 0;
+    class EffectWithInitAction implements OnInitEffects, OnIdentifyEffects {
+      effectId = '';
+      ngrxOnIdentifyEffects(): string {
+        return this.effectId;
+      }
+      ngrxOnInitEffects() {
+        return { type: '[EffectWithInitAction] Init' };
+      }
+      constructor() {
+        this.effectId = (id++).toString();
+      }
+    }
+    const store = TestBed.get(Store);
+
+    effectSources.addEffects(new EffectWithInitAction());
+    effectSources.addEffects(new EffectWithInitAction());
+
+    expect(store.dispatch).toHaveBeenCalledTimes(2);
+  });
+
   describe('toActions() Operator', () => {
     describe('with @Effect()', () => {
       const a = { type: 'From Source A' };

modules/effects/spec/integration.spec.ts

@@ -49,7 +49,7 @@ describe('NgRx Effects Integration spec', () => {
   });
 
   it('should dispatch init actions in the correct order', () => {
-    expect(dispatch.calls.count()).toBe(7);
+    expect(dispatch.calls.count()).toBe(6);
 
     // All of the root effects init actions are dispatched first
     expect(dispatch.calls.argsFor(0)).toEqual([
@@ -73,11 +73,6 @@ describe('NgRx Effects Integration spec', () => {
     expect(dispatch.calls.argsFor(5)).toEqual([
       { type: '[FeatEffectWithIdentifierAndInitAction]: INIT' },
     ]);
-
-    // While the effect has the same identifier the init effect action is still being dispatched
-    expect(dispatch.calls.argsFor(6)).toEqual([
-      { type: '[FeatEffectWithIdentifierAndInitAction]: INIT' },
-    ]);
   });
 
   it('throws if forRoot() is used more than once', (done: DoneFn) => {

modules/effects/src/effect_sources.ts

@@ -8,6 +8,7 @@ import {
   groupBy,
   map,
   mergeMap,
+  tap,
 } from 'rxjs/operators';
 
 import {
@@ -31,13 +32,6 @@ export class EffectSources extends Subject<any> {
 
   addEffects(effectSourceInstance: any): void {
     this.next(effectSourceInstance);
-
-    if (
-      onInitEffects in effectSourceInstance &&
-      typeof effectSourceInstance[onInitEffects] === 'function'
-    ) {
-      this.store.dispatch(effectSourceInstance[onInitEffects]());
-    }
   }
 
   /**
@@ -46,7 +40,19 @@ export class EffectSources extends Subject<any> {
   toActions(): Observable<Action> {
     return this.pipe(
       groupBy(getSourceForInstance),
-      mergeMap(source$ => source$.pipe(groupBy(effectsInstance))),
+      mergeMap(source$ => {
+        return source$.pipe(
+          groupBy(effectsInstance),
+          tap(() => {
+            if (
+              onInitEffects in source$.key &&
+              typeof source$.key[onInitEffects] === 'function'
+            ) {
+              this.store.dispatch(source$.key.ngrxOnInitEffects());
+            }
+          })
+        );
+      }),
       mergeMap(source$ =>
         source$.pipe(
           exhaustMap(resolveEffectSource(this.errorHandler)),