Merge pull request #73 from ntziolis/fix/ng-directive-unregister

fix(effects-directive): unregister all effects of a provider
ngneat · Jul 1, 2024 · ce07658 · ce07658
2 parents da32a41 + 56ddefb
commit ce07658
Show file tree

Hide file tree

Showing 2 changed files with 35 additions and 10 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
@@ -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()
@@ -54,6 +60,7 @@ describe('provideDirectiveEffects & EffectsDirective', () => {
   beforeEach(() => {
     spy.mockClear();
     spy2.mockClear();
+    spy3.mockClear();
   });
 
   it('should provide effects', () => {
@@ -95,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', () => {
@@ -116,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', () => {
@@ -148,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', () => {

diff --git 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<ProvidedEffectToken>
   >();
 
   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<ProvidedEffectToken>)
+          : [];
+
+        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<Effect[]>((effects, [sourceInstance, token]) => {
-      const effect = getProvidedEffect(sourceInstance, token);
+    ].reduce<Effect[]>((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;