feat(Effects): Ensure effects are only subscribed to once

Author: elwynelwyn

build/util.ts

@@ -85,7 +85,7 @@ export async function ignoreErrors<T>(promise: Promise<T>): Promise<T | null> {
 }
 
 export function fromNpm(command: string) {
-  return `./node_modules/.bin/${command}`;
+  return path.normalize(`./node_modules/.bin/${command}`);
 }
 
 export function getPackageFilePath(pkg: string, filename: string) {

docs/effects/README.md

@@ -36,7 +36,19 @@ store. The `ofType` operator lets you filter for actions of a certain type in wh
 want to use to perform a side effect.
 
 ## Example
-1. Create an AuthEffects service that describes a source of login actions:
+1. Register the EffectsModule in your application root imports:
+```ts
+import { EffectsModule } from '@ngrx/effects';
+
+@NgModule({
+  imports: [
+    EffectsModule.forRoot()
+  ]
+})
+export class AppModule { }
+```
+
+2. Create an AuthEffects service that describes a source of login actions:
 
 ```ts
 // ./effects/auth.ts
@@ -70,7 +82,7 @@ export class AuthEffects {
 }
 ```
 
-2. Register your effects via `EffectsModule.run` method in your module's `imports`:
+3. Register your effects via `EffectsModule.run` method in your module's `imports`:
 
 ```ts
 import { EffectsModule } from '@ngrx/effects';
@@ -81,7 +93,7 @@ import { AuthEffects } from './effects/auth';
     EffectsModule.run(AuthEffects)
   ]
 })
-export class AppModule { }
+export class YourAuthModule { }
 ```
 
 ## API Documentation

docs/effects/api.md

@@ -4,11 +4,19 @@
 
 Feature module for @ngrx/effects.
 
+### forRoot
+Registers internal @ngrx/effects services to run in your application.
+This is required once in your root app module.
+
 ### run
 
 Registers an effects class to run when the target module bootstraps.
 Call once per each effect class you want to run.
 
+Note that running an effects class multiple times (for example via
+different lazy loaded modules) will not cause Effects to run multiple
+times.
+
 Usage:
 ```ts
 @NgModule({

modules/effects/spec/effects-subscription.spec.ts

@@ -2,22 +2,25 @@ import { ReflectiveInjector } from '@angular/core';
 import { of } from 'rxjs/observable/of';
 import { Effect } from '../src/effects';
 import { EffectsSubscription } from '../src/effects-subscription';
+import { SingletonEffectsService } from '../src/singleton-effects.service';
 
 
 describe('Effects Subscription', () => {
   it('should add itself to a parent subscription if one exists', () => {
     const observer: any = { next() { } };
-    const root = new EffectsSubscription(observer, undefined, undefined);
+    const singletonEffectsService = new SingletonEffectsService();
+    const root = new EffectsSubscription(observer, singletonEffectsService, undefined, undefined);
 
     spyOn(root, 'add');
-    const child = new EffectsSubscription(observer, root, undefined);
+    const child = new EffectsSubscription(observer, singletonEffectsService, root, undefined);
 
     expect(root.add).toHaveBeenCalledWith(child);
   });
 
   it('should unsubscribe for all effects when destroyed', () => {
     const observer: any = { next() { } };
-    const subscription = new EffectsSubscription(observer, undefined, undefined);
+    const singletonEffectsService = new SingletonEffectsService();
+    const subscription = new EffectsSubscription(observer, singletonEffectsService, undefined, undefined);
 
     spyOn(subscription, 'unsubscribe');
     subscription.ngOnDestroy();
@@ -33,12 +36,32 @@ describe('Effects Subscription', () => {
     }
     const instance = new Source();
     const observer: any = { next: jasmine.createSpy('next') };
+    const singletonEffectsService = new SingletonEffectsService();
 
-    const subscription = new EffectsSubscription(observer, undefined, [ instance ]);
+    const subscription = new EffectsSubscription(observer, singletonEffectsService, undefined, [ instance ]);
 
     expect(observer.next).toHaveBeenCalledTimes(3);
     expect(observer.next).toHaveBeenCalledWith('a');
     expect(observer.next).toHaveBeenCalledWith('b');
     expect(observer.next).toHaveBeenCalledWith('c');
   });
-});
+
+  it('should not merge duplicate effects instances when a SingletonEffectsService is provided', () => {
+    class Source {
+      @Effect() a$ = of('a');
+      @Effect() b$ = of('b');
+      @Effect() c$ = of('c');
+    }
+    const instance = new Source();
+    const observer: any = { next: jasmine.createSpy('next') };
+    const singletonEffectsService = new SingletonEffectsService();
+    singletonEffectsService.removeExistingAndRegisterNew([ instance ]);
+
+    const subscription = new EffectsSubscription(observer, singletonEffectsService, undefined, [ instance ]);
+
+    expect(observer.next).not.toHaveBeenCalled();
+    expect(observer.next).not.toHaveBeenCalledWith('a');
+    expect(observer.next).not.toHaveBeenCalledWith('b');
+    expect(observer.next).not.toHaveBeenCalledWith('c');
+  });
+});

modules/effects/spec/singleton-effects.service.spec.ts

@@ -0,0 +1,32 @@
+import { of } from 'rxjs/observable/of';
+import { Effect } from '../src/effects';
+import { SingletonEffectsService } from '../src/singleton-effects.service';
+
+describe('SingletonEffectsService', () => {
+  it('should filter out duplicate effect instances and register new ones', () => {
+    class Source1 {
+      @Effect() a$ = of('a');
+      @Effect() b$ = of('b');
+      @Effect() c$ = of('c');
+    }
+    class Source2 {
+      @Effect() d$ = of('d');
+      @Effect() e$ = of('e');
+      @Effect() f$ = of('f');
+    }
+    const instance1 = new Source1();
+    const instance2 = new Source2();
+    let singletonEffectsService = new SingletonEffectsService();
+
+    let result = singletonEffectsService.removeExistingAndRegisterNew([ instance1 ]);
+    expect(result).toContain(instance1);
+
+    result = singletonEffectsService.removeExistingAndRegisterNew([ instance1, instance2 ]);
+    expect(result).not.toContain(instance1);
+    expect(result).toContain(instance2);
+
+    result = singletonEffectsService.removeExistingAndRegisterNew([ instance1, instance2 ]);
+    expect(result).not.toContain(instance1);
+    expect(result).not.toContain(instance2);
+  });
+});

modules/effects/src/effects-subscription.ts

@@ -4,6 +4,7 @@ import { Observer } from 'rxjs/Observer';
 import { Subscription } from 'rxjs/Subscription';
 import { merge } from 'rxjs/observable/merge';
 import { mergeEffects } from './effects';
+import { SingletonEffectsService } from './singleton-effects.service';
 
 
 export const effects = new OpaqueToken('ngrx/effects: Effects');
@@ -12,6 +13,7 @@ export const effects = new OpaqueToken('ngrx/effects: Effects');
 export class EffectsSubscription extends Subscription implements OnDestroy {
   constructor(
     @Inject(Store) private store: Observer<Action>,
+    @Inject(SingletonEffectsService) private singletonEffectsService: SingletonEffectsService,
     @Optional() @SkipSelf() public parent?: EffectsSubscription,
     @Optional() @Inject(effects) effectInstances?: any[]
   ) {
@@ -27,6 +29,8 @@ export class EffectsSubscription extends Subscription implements OnDestroy {
   }
 
   addEffects(effectInstances: any[]) {
+    effectInstances = this.singletonEffectsService.removeExistingAndRegisterNew(effectInstances);
+
     const sources = effectInstances.map(mergeEffects);
     const merged = merge(...sources);
 

modules/effects/src/effects.module.ts

@@ -2,6 +2,7 @@ import { NgModule, Injector, Type, APP_BOOTSTRAP_LISTENER, OpaqueToken } from '@
 import { Actions } from './actions';
 import { EffectsSubscription, effects } from './effects-subscription';
 import { runAfterBootstrapEffects, afterBootstrapEffects } from './bootstrap-listener';
+import { SingletonEffectsService } from './singleton-effects.service';
 
 
 @NgModule({
@@ -17,6 +18,15 @@ import { runAfterBootstrapEffects, afterBootstrapEffects } from './bootstrap-lis
   ]
 })
 export class EffectsModule {
+  static forRoot() {
+    return {
+      ngModule: EffectsModule,
+      providers: [
+        SingletonEffectsService
+      ]
+    };
+  }
+
   static run(type: Type<any>) {
     return {
       ngModule: EffectsModule,

modules/effects/src/singleton-effects.service.ts

@@ -0,0 +1,17 @@
+import { Injectable } from '@angular/core';
+
+@Injectable()
+export class SingletonEffectsService {
+  private registeredEffects: string[] = [];
+
+  removeExistingAndRegisterNew (effectInstances: any[]): any[] {
+    return effectInstances.filter(instance => {
+      const instanceAsString = instance.constructor.toString();
+      if (this.registeredEffects.indexOf(instanceAsString) === -1) {
+        this.registeredEffects.push(instanceAsString);
+        return true;
+      }
+      return false;
+    });
+  }
+}