refactor(Effects): use EffectIdentifier hook

modules/effects/spec/effect_sources.spec.ts

@@ -1,10 +1,10 @@
 import { ErrorHandler } from '@angular/core';
 import { TestBed } from '@angular/core/testing';
 import { cold, getTestScheduler } from 'jasmine-marbles';
-import { concat, empty, NEVER, Observable, of, throwError, timer } from 'rxjs';
+import { concat, NEVER, Observable, of, throwError, timer } from 'rxjs';
 import { map } from 'rxjs/operators';
 
-import { Effect, EffectSources } from '../';
+import { Effect, EffectSources, EffectIdentifier } from '../';
 
 describe('EffectSources', () => {
   let mockErrorReporter: ErrorHandler;
@@ -27,22 +27,7 @@ describe('EffectSources', () => {
 
     effectSources.addEffects(effectSource);
 
-    expect(effectSources.next).toHaveBeenCalledWith({
-      effectSourceInstance: effectSource,
-      identifier: {},
-    });
-  });
-
-  it('should be possible add an identifier to "addEffects"', () => {
-    const effectSource = {};
-    spyOn(effectSources, 'next');
-
-    effectSources.addEffects(effectSource, 'custom-identifier');
-
-    expect(effectSources.next).toHaveBeenCalledWith({
-      effectSourceInstance: effectSource,
-      identifier: 'custom-identifier',
-    });
+    expect(effectSources.next).toHaveBeenCalledWith(effectSource);
   });
 
   describe('toActions() Operator', () => {
@@ -52,6 +37,7 @@ describe('EffectSources', () => {
     const d = { not: 'a valid action' };
     const e = undefined;
     const f = null;
+    const i = { type: 'From Source Identifier' };
 
     let circularRef = {} as any;
     circularRef.circularRef = circularRef;
@@ -97,10 +83,17 @@ describe('EffectSources', () => {
       never = timer(50, getTestScheduler() as any).pipe(map(() => 'update'));
     }
 
+    class SourceWithIdentifier implements EffectIdentifier {
+      identifier: string;
+      @Effect() i$ = alwaysOf(i);
+
+      constructor(identifier: string) {
+        this.identifier = identifier;
+      }
+    }
+
     it('should resolve effects from instances', () => {
-      const sources$ = cold('--a--', {
-        a: { effectSourceInstance: new SourceA() },
-      });
+      const sources$ = cold('--a--', { a: new SourceA() });
       const expected = cold('--a--', { a });
 
       const output = toActions(sources$);
@@ -110,7 +103,7 @@ describe('EffectSources', () => {
 
     it('should ignore duplicate sources', () => {
       const sources$ = cold('--a--a--a--', {
-        a: { effectSourceInstance: new SourceA() },
+        a: new SourceA(),
       });
       const expected = cold('--a--------', { a });
 
@@ -119,34 +112,34 @@ describe('EffectSources', () => {
       expect(output).toBeObservable(expected);
     });
 
-    it('should ignore sources with the same identifier', () => {
+    it('should resolve effects with different identifiers', () => {
       const sources$ = cold('--a--b--c--', {
-        a: { effectSourceInstance: new SourceA(), identifier: 'a' },
-        b: { effectSourceInstance: new SourceB(), identifier: 'a' },
-        c: { effectSourceInstance: new SourceC(), identifier: 'a' },
+        a: new SourceWithIdentifier('a'),
+        b: new SourceWithIdentifier('b'),
+        c: new SourceWithIdentifier('c'),
       });
-      const expected = cold('--a--------', { a });
+      const expected = cold('--i--i--i--', { i });
 
       const output = toActions(sources$);
 
       expect(output).toBeObservable(expected);
     });
 
-    it('should resolve effects from same class but with different identifiers', () => {
+    it('should ignore effects with the same identifier', () => {
       const sources$ = cold('--a--b--c--', {
-        a: { effectSourceInstance: new SourceA(), identifier: 'a' },
-        b: { effectSourceInstance: new SourceA(), identifier: 'b' },
-        c: { effectSourceInstance: new SourceA(), identifier: 'c' },
+        a: new SourceWithIdentifier('a'),
+        b: new SourceWithIdentifier('a'),
+        c: new SourceWithIdentifier('a'),
       });
-      const expected = cold('--a--a--a--', { a });
+      const expected = cold('--i--------', { i });
 
       const output = toActions(sources$);
 
       expect(output).toBeObservable(expected);
     });
 
     it('should report an error if an effect dispatches an invalid action', () => {
-      const sources$ = of({ effectSourceInstance: new SourceD() });
+      const sources$ = of(new SourceD());
 
       toActions(sources$).subscribe();
 
@@ -158,7 +151,7 @@ describe('EffectSources', () => {
     });
 
     it('should report an error if an effect dispatches an `undefined`', () => {
-      const sources$ = of({ effectSourceInstance: new SourceE() });
+      const sources$ = of(new SourceE());
 
       toActions(sources$).subscribe();
 
@@ -168,7 +161,7 @@ describe('EffectSources', () => {
     });
 
     it('should report an error if an effect dispatches a `null`', () => {
-      const sources$ = of({ effectSourceInstance: new SourceF() });
+      const sources$ = of(new SourceF());
 
       toActions(sources$).subscribe();
 
@@ -178,7 +171,7 @@ describe('EffectSources', () => {
     });
 
     it(`should not break when the action in the error message can't be stringified`, () => {
-      const sources$ = of({ effectSourceInstance: new SourceG() });
+      const sources$ = of(new SourceG());
 
       toActions(sources$).subscribe();
 
@@ -191,7 +184,7 @@ describe('EffectSources', () => {
 
     it('should not complete the group if just one effect completes', () => {
       const sources$ = cold('g', {
-        g: { effectSourceInstance: new SourceH() },
+        g: new SourceH(),
       });
       const expected = cold('a----b-----', { a: 'value', b: 'update' });
 

modules/effects/src/effect_sources.ts

@@ -15,32 +15,24 @@ import { resolveEffectSource } from './effects_resolver';
 import { getSourceForInstance } from './effects_metadata';
 
 @Injectable()
-export class EffectSources extends Subject<{
-  identifier: any;
-  effectSourceInstance: any;
-}> {
+export class EffectSources extends Subject<any> {
   constructor(private errorHandler: ErrorHandler) {
     super();
   }
 
-  addEffects(
-    effectSourceInstance: any,
-    identifier: any = getSourceForInstance(effectSourceInstance)
-  ) {
-    this.next({ identifier, effectSourceInstance });
+  addEffects(effectSourceInstance: any) {
+    this.next(effectSourceInstance);
   }
 
   /**
    * @internal
    */
   toActions(): Observable<Action> {
     return this.pipe(
-      groupBy(source => source.identifier),
+      groupBy(source => source.identifier || getSourceForInstance(source)),
       mergeMap(source$ =>
         source$.pipe(
-          exhaustMap(source =>
-            resolveEffectSource(source.effectSourceInstance)
-          ),
+          exhaustMap(resolveEffectSource),
           map(output => {
             verifyOutput(output, this.errorHandler);
 

modules/effects/src/index.ts

@@ -11,3 +11,4 @@ export { OnRunEffects } from './on_run_effects';
 export { EffectNotification } from './effect_notification';
 export { ROOT_EFFECTS_INIT } from './effects_root_module';
 export { UPDATE_EFFECTS, UpdateEffects } from './effects_feature_module';
+export { EffectIdentifier } from './lifecycle_hooks';

modules/effects/src/lifecycle_hooks.ts

@@ -0,0 +1,6 @@
+export interface EffectIdentifier {
+  /**
+   * Used to differentiate effect instances
+   */
+  identifier: string;
+}