diff --git a/modules/effects/spec/effect_sources.spec.ts b/modules/effects/spec/effect_sources.spec.ts index 983f50f9e6..c0f26efa64 100644 --- a/modules/effects/spec/effect_sources.spec.ts +++ b/modules/effects/spec/effect_sources.spec.ts @@ -1,10 +1,11 @@ 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 { EffectIdentifier } from '../src'; describe('EffectSources', () => { let mockErrorReporter: ErrorHandler; @@ -27,22 +28,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 +38,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 +84,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 +104,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,26 +113,26 @@ 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$); @@ -146,7 +140,7 @@ describe('EffectSources', () => { }); 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 +152,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 +162,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 +172,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 +185,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' }); diff --git a/modules/effects/src/effect_sources.ts b/modules/effects/src/effect_sources.ts index 8793799fbb..662b9b8052 100644 --- a/modules/effects/src/effect_sources.ts +++ b/modules/effects/src/effect_sources.ts @@ -15,19 +15,13 @@ 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 { constructor(private errorHandler: ErrorHandler) { super(); } - addEffects( - effectSourceInstance: any, - identifier: any = getSourceForInstance(effectSourceInstance) - ) { - this.next({ identifier, effectSourceInstance }); + addEffects(effectSourceInstance: any) { + this.next(effectSourceInstance); } /** @@ -35,12 +29,12 @@ export class EffectSources extends Subject<{ */ toActions(): Observable { return this.pipe( - groupBy(source => source.identifier), + groupBy(source => { + return source.identifier || getSourceForInstance(source); + }), mergeMap(source$ => source$.pipe( - exhaustMap(source => - resolveEffectSource(source.effectSourceInstance) - ), + exhaustMap(resolveEffectSource), map(output => { verifyOutput(output, this.errorHandler); diff --git a/modules/effects/src/index.ts b/modules/effects/src/index.ts index da9f9e4180..9efe234721 100644 --- a/modules/effects/src/index.ts +++ b/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'; diff --git a/modules/effects/src/lifecycle_hooks.ts b/modules/effects/src/lifecycle_hooks.ts new file mode 100644 index 0000000000..ed011b2538 --- /dev/null +++ b/modules/effects/src/lifecycle_hooks.ts @@ -0,0 +1,6 @@ +export interface EffectIdentifier { + /** + * Used to differentiate effect instances + */ + identifier: string; +}