From 12bd426bae0e222b8f44ba6db268261f5e2156a6 Mon Sep 17 00:00:00 2001 From: ntziolis Date: Mon, 1 Jul 2024 06:55:50 +0000 Subject: [PATCH 1/2] fix(effects-directive): unregister all effects of a provider --- .../src/lib/use-directive-effects.spec.ts | 6 +++++ .../src/lib/use-directive-effects.ts | 26 ++++++++++++++----- 2 files changed, 25 insertions(+), 7 deletions(-) diff --git a/libs/effects-ng/src/lib/use-directive-effects.spec.ts b/libs/effects-ng/src/lib/use-directive-effects.spec.ts index 1cc6f6a..262b6e4 100644 --- a/libs/effects-ng/src/lib/use-directive-effects.spec.ts +++ b/libs/effects-ng/src/lib/use-directive-effects.spec.ts @@ -13,14 +13,20 @@ import { provideEffects } from './provide-effects'; const spy = jest.fn(); const spy2 = jest.fn(); +const spy3 = jest.fn(); const loadTodos = createAction('[Todos] Load Todos'); const loadTodos2 = createAction('[Todos] Load Todos 2'); +const loadTodos3 = createAction('[Todos] Load Todos 3'); @Injectable() class EffectsOne { loadTodos$ = createEffect((actions) => actions.pipe(ofType(loadTodos), tap(spy)) ); + + loadTodos3$ = createEffect((actions) => + actions.pipe(ofType(loadTodos3), tap(spy3)) + ); } @Injectable() diff --git a/libs/effects-ng/src/lib/use-directive-effects.ts b/libs/effects-ng/src/lib/use-directive-effects.ts index a6143aa..f1c26af 100644 --- a/libs/effects-ng/src/lib/use-directive-effects.ts +++ b/libs/effects-ng/src/lib/use-directive-effects.ts @@ -42,7 +42,7 @@ export class EffectsDirective implements OnDestroy { private readonly manager = inject(EFFECTS_MANAGER, { optional: true }); private readonly sourceInstancesWithProvidersEffectsTokens = new Map< any, - ProvidedEffectToken + Array >(); constructor() { @@ -67,9 +67,19 @@ export class EffectsDirective implements OnDestroy { const sourceInstance = Object.getPrototypeOf(instance); const token = generateProvidedEffectToken(provider, key); + const tokens = this.sourceInstancesWithProvidersEffectsTokens.has( + sourceInstance + ) + ? (this.sourceInstancesWithProvidersEffectsTokens.get( + sourceInstance + ) as Array) + : []; + + tokens.push(token); + this.sourceInstancesWithProvidersEffectsTokens.set( Object.getPrototypeOf(instance), - token + tokens ); if (isEffectProvided(sourceInstance, token)) { @@ -94,13 +104,15 @@ export class EffectsDirective implements OnDestroy { private unregisterEffect(): void { const effects = [ ...this.sourceInstancesWithProvidersEffectsTokens.entries(), - ].reduce((effects, [sourceInstance, token]) => { - const effect = getProvidedEffect(sourceInstance, token); + ].reduce((effects, [sourceInstance, tokens]) => { + for (const token of tokens) { + const effect = getProvidedEffect(sourceInstance, token); - decreaseProvidedEffectSources(sourceInstance, token); + decreaseProvidedEffectSources(sourceInstance, token); - if (effect && !isEffectProvided(sourceInstance, token)) { - effects.push(effect); + if (effect && !isEffectProvided(sourceInstance, token)) { + effects.push(effect); + } } return effects; From 56ddefbdf5f057342509c0f7e87eb9b6b5df435f Mon Sep 17 00:00:00 2001 From: ntziolis Date: Mon, 1 Jul 2024 09:49:10 +0000 Subject: [PATCH 2/2] feat(effects-directive): extended test cases --- .../src/lib/use-directive-effects.spec.ts | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/libs/effects-ng/src/lib/use-directive-effects.spec.ts b/libs/effects-ng/src/lib/use-directive-effects.spec.ts index 262b6e4..751aee8 100644 --- a/libs/effects-ng/src/lib/use-directive-effects.spec.ts +++ b/libs/effects-ng/src/lib/use-directive-effects.spec.ts @@ -60,6 +60,7 @@ describe('provideDirectiveEffects & EffectsDirective', () => { beforeEach(() => { spy.mockClear(); spy2.mockClear(); + spy3.mockClear(); }); it('should provide effects', () => { @@ -101,6 +102,8 @@ describe('provideDirectiveEffects & EffectsDirective', () => { TestBed.createComponent(componentType); expect(spy).toHaveBeenCalled(); + expect(spy2).not.toHaveBeenCalled(); + expect(spy3).not.toHaveBeenCalled(); }); it('should subscribe on the same effects only once', () => { @@ -122,6 +125,7 @@ describe('provideDirectiveEffects & EffectsDirective', () => { expect(spy).toHaveBeenCalledTimes(2); expect(spy2).toHaveBeenCalledTimes(1); + expect(spy3).not.toHaveBeenCalled(); }); it('should unsubscribe only from effects that was registered via provideDirectiveEffects when component is destroyed', () => { @@ -154,25 +158,28 @@ describe('provideDirectiveEffects & EffectsDirective', () => { const fixture3 = TestBed.createComponent(TestComponent); const actions = TestBed.inject(Actions); - actions.dispatch(loadTodos2()); + actions.dispatch(loadTodos2(), loadTodos3()); expect(spy).toHaveBeenCalledTimes(3); expect(spy2).toHaveBeenCalledTimes(1); + expect(spy2).toHaveBeenCalledTimes(1); fixture.destroy(); fixture2.destroy(); - actions.dispatch(loadTodos(), loadTodos2()); + actions.dispatch(loadTodos(), loadTodos2(), loadTodos3()); expect(spy).toHaveBeenCalledTimes(4); expect(spy2).toHaveBeenCalledTimes(2); + expect(spy3).toHaveBeenCalledTimes(2); fixture3.destroy(); - actions.dispatch(loadTodos(), loadTodos2()); + actions.dispatch(loadTodos(), loadTodos2(), loadTodos3()); expect(spy).toHaveBeenCalledTimes(4); expect(spy2).toHaveBeenCalledTimes(3); + expect(spy3).toHaveBeenCalledTimes(2); }); it('should properly determine source instances and manage their effects', () => {