From 720ec774dac2a46179a2a2b890955f21c78e8c98 Mon Sep 17 00:00:00 2001
From: Rainer Hahnekamp <rainer.hahnekamp@gmail.com>
Date: Sun, 22 Sep 2024 13:39:33 +0200
Subject: [PATCH 1/9] fix(signals): use `Injector` of `rxMethod` instance
 caller

Ensure that an instance of `rxMethod` uses the `Injector` of the caller where
the instance was invoked, rather than the injector of the node where `rxMethod`
was created.

This change addresses a potential memory leak where a `SignalStore` method,
generated by `rxMethod` and provided at the root level, continues to track a
signal via its `effect` using the `RootInjector`. In cases where the component
calling the method is destroyed, the `Signal` remains tracked, leading to
unintended behavior and memory retention.

With this commit, `rxMethod` now prioritizes the injector of the instance
caller, falling back to the creator's injector only if no injection context
exists. Additionally, a custom injector can be provided by the caller and will
take precedence when specified.

For `Observable`, since it has a different completion mechanism, the instance
caller is still responsible for handling unsubscription or completion.
---
 .../rxjs-interop/spec/rx-method.spec.ts       | 127 +++++++++++++++++-
 modules/signals/rxjs-interop/src/rx-method.ts |  36 ++++-
 2 files changed, 155 insertions(+), 8 deletions(-)

diff --git a/modules/signals/rxjs-interop/spec/rx-method.spec.ts b/modules/signals/rxjs-interop/spec/rx-method.spec.ts
index a2e594943c..c31b620599 100644
--- a/modules/signals/rxjs-interop/spec/rx-method.spec.ts
+++ b/modules/signals/rxjs-interop/spec/rx-method.spec.ts
@@ -1,13 +1,20 @@
 import {
+  Component,
   createEnvironmentInjector,
   EnvironmentInjector,
+  inject,
   Injectable,
+  Injector,
+  OnInit,
   signal,
 } from '@angular/core';
 import { TestBed } from '@angular/core/testing';
-import { BehaviorSubject, pipe, Subject, tap } from 'rxjs';
+import { BehaviorSubject, finalize, pipe, Subject, tap } from 'rxjs';
 import { rxMethod } from '../src';
 import { createLocalService } from '../../spec/helpers';
+import { provideRouter } from '@angular/router';
+import { provideLocationMocks } from '@angular/common/testing';
+import { RouterTestingHarness } from '@angular/router/testing';
 
 describe('rxMethod', () => {
   it('runs with a value', () => {
@@ -231,4 +238,122 @@ describe('rxMethod', () => {
     TestBed.flushEffects();
     expect(counter()).toBe(4);
   });
+
+  it('completes on manual destroy with Signals', () => {
+    TestBed.runInInjectionContext(() => {
+      let completed = false;
+      const counter = signal(1);
+      const fn = rxMethod<number>(finalize(() => (completed = true)));
+      TestBed.flushEffects();
+      fn(counter);
+      fn.unsubscribe();
+      expect(completed).toBe(true);
+    });
+  });
+
+  describe('Signals and effect injector', () => {
+    @Injectable({ providedIn: 'root' })
+    class GlobalService {
+      rxMethodStatus = 'none';
+      log = rxMethod<string>(
+        pipe(
+          tap({
+            next: () => (this.rxMethodStatus = 'started'),
+            finalize: () => (this.rxMethodStatus = 'destroyed'),
+          })
+        )
+      );
+    }
+
+    @Component({
+      selector: `app-storeless`,
+      template: ``,
+      standalone: true,
+    })
+    class WithoutStoreComponent {}
+
+    function setup(WithStoreComponent: new () => unknown): GlobalService {
+      TestBed.configureTestingModule({
+        providers: [
+          provideRouter([
+            { path: 'with-store', component: WithStoreComponent },
+            {
+              path: 'without-store',
+              component: WithoutStoreComponent,
+            },
+          ]),
+          provideLocationMocks(),
+        ],
+      });
+
+      return TestBed.inject(GlobalService);
+    }
+
+    it('should destroy with component injector when rxMethod is in root and RxMethod in component', async () => {
+      @Component({
+        selector: 'app-with-store',
+        template: ``,
+        standalone: true,
+      })
+      class WithStoreComponent {
+        store = inject(GlobalService);
+
+        constructor() {
+          this.store.log(signal('test'));
+        }
+      }
+
+      const globalService = setup(WithStoreComponent);
+
+      const harness = await RouterTestingHarness.create('/with-store');
+      expect(globalService.rxMethodStatus).toBe('started');
+      await harness.navigateByUrl('/without-store');
+      expect(globalService.rxMethodStatus).toBe('destroyed');
+    });
+
+    it("should fallback to rxMethod's injector when RxMethod's call is outside of injection context", async () => {
+      @Component({
+        selector: `app-store`,
+        template: ``,
+        standalone: true,
+      })
+      class WithStoreComponent implements OnInit {
+        store = inject(GlobalService);
+
+        ngOnInit() {
+          this.store.log(signal('test'));
+        }
+      }
+
+      const globalService = setup(WithStoreComponent);
+
+      const harness = await RouterTestingHarness.create('/with-store');
+      expect(globalService.rxMethodStatus).toBe('started');
+      await harness.navigateByUrl('/without-store');
+      expect(globalService.rxMethodStatus).toBe('started');
+    });
+
+    it('should provide the injector for RxMethod on call', async () => {
+      @Component({
+        selector: `app-store`,
+        template: ``,
+        standalone: true,
+      })
+      class WithStoreComponent implements OnInit {
+        store = inject(GlobalService);
+        injector = inject(Injector);
+
+        ngOnInit() {
+          this.store.log(signal('test'), this.injector);
+        }
+      }
+
+      const globalService = setup(WithStoreComponent);
+
+      const harness = await RouterTestingHarness.create('/with-store');
+      expect(globalService.rxMethodStatus).toBe('started');
+      await harness.navigateByUrl('/without-store');
+      expect(globalService.rxMethodStatus).toBe('destroyed');
+    });
+  });
 });
diff --git a/modules/signals/rxjs-interop/src/rx-method.ts b/modules/signals/rxjs-interop/src/rx-method.ts
index d31c075bf9..d93e57d213 100644
--- a/modules/signals/rxjs-interop/src/rx-method.ts
+++ b/modules/signals/rxjs-interop/src/rx-method.ts
@@ -11,7 +11,8 @@ import {
 import { isObservable, noop, Observable, Subject, Unsubscribable } from 'rxjs';
 
 type RxMethod<Input> = ((
-  input: Input | Signal<Input> | Observable<Input>
+  input: Input | Signal<Input> | Observable<Input>,
+  injector?: Injector
 ) => Unsubscribable) &
   Unsubscribable;
 
@@ -24,22 +25,32 @@ export function rxMethod<Input>(
   }
 
   const injector = config?.injector ?? inject(Injector);
-  const destroyRef = injector.get(DestroyRef);
   const source$ = new Subject<Input>();
-
   const sourceSub = generator(source$).subscribe();
-  destroyRef.onDestroy(() => sourceSub.unsubscribe());
 
-  const rxMethodFn = (input: Input | Signal<Input> | Observable<Input>) => {
+  const rxMethodFn = (
+    input: Input | Signal<Input> | Observable<Input>,
+    customInjector?: Injector
+  ) => {
     if (isSignal(input)) {
+      const callerInjector = getCallerInjectorIfAvailable();
+      const instanceInjector = customInjector ?? callerInjector ?? injector;
+
       const watcher = effect(
         () => {
           const value = input();
           untracked(() => source$.next(value));
         },
-        { injector }
+        { injector: instanceInjector }
       );
-      const instanceSub = { unsubscribe: () => watcher.destroy() };
+
+      instanceInjector.get(DestroyRef).onDestroy(() => {
+        sourceSub.unsubscribe();
+      });
+
+      const instanceSub = {
+        unsubscribe: () => watcher.destroy(),
+      };
       sourceSub.add(instanceSub);
 
       return instanceSub;
@@ -49,6 +60,9 @@ export function rxMethod<Input>(
       const instanceSub = input.subscribe((value) => source$.next(value));
       sourceSub.add(instanceSub);
 
+      const destroyRef = injector.get(DestroyRef);
+      destroyRef.onDestroy(() => sourceSub.unsubscribe());
+
       return instanceSub;
     }
 
@@ -59,3 +73,11 @@ export function rxMethod<Input>(
 
   return rxMethodFn;
 }
+
+function getCallerInjectorIfAvailable(): Injector | null {
+  try {
+    return inject(Injector);
+  } catch (e) {
+    return null;
+  }
+}

From 0e32f319cb4244e5bfbe8cecbbae997166e0c7b4 Mon Sep 17 00:00:00 2001
From: Rainer Hahnekamp <rainer.hahnekamp@gmail.com>
Date: Tue, 24 Sep 2024 14:46:42 +0200
Subject: [PATCH 2/9] fix: rename tests to existing pattern

---
 modules/signals/rxjs-interop/spec/rx-method.spec.ts | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/modules/signals/rxjs-interop/spec/rx-method.spec.ts b/modules/signals/rxjs-interop/spec/rx-method.spec.ts
index c31b620599..419994dfc0 100644
--- a/modules/signals/rxjs-interop/spec/rx-method.spec.ts
+++ b/modules/signals/rxjs-interop/spec/rx-method.spec.ts
@@ -289,7 +289,7 @@ describe('rxMethod', () => {
       return TestBed.inject(GlobalService);
     }
 
-    it('should destroy with component injector when rxMethod is in root and RxMethod in component', async () => {
+    it('destroys with component injector when rxMethod is in root and RxMethod in component', async () => {
       @Component({
         selector: 'app-with-store',
         template: ``,
@@ -311,7 +311,7 @@ describe('rxMethod', () => {
       expect(globalService.rxMethodStatus).toBe('destroyed');
     });
 
-    it("should fallback to rxMethod's injector when RxMethod's call is outside of injection context", async () => {
+    it("falls back to rxMethod's injector when RxMethod's call is outside of injection context", async () => {
       @Component({
         selector: `app-store`,
         template: ``,
@@ -333,7 +333,7 @@ describe('rxMethod', () => {
       expect(globalService.rxMethodStatus).toBe('started');
     });
 
-    it('should provide the injector for RxMethod on call', async () => {
+    it('provides the injector for RxMethod on call', async () => {
       @Component({
         selector: `app-store`,
         template: ``,

From 93cb7e12758dc1a841236ab0064208b98efa8e21 Mon Sep 17 00:00:00 2001
From: Rainer Hahnekamp <rainer.hahnekamp@gmail.com>
Date: Tue, 24 Sep 2024 19:36:45 +0200
Subject: [PATCH 3/9] fix: revert changes to the old destroy mechanism, except
 the `instanceInjector`

---
 .../rxjs-interop/spec/rx-method.spec.ts       | 86 +++++++++++++++----
 modules/signals/rxjs-interop/src/rx-method.ts | 15 +---
 2 files changed, 71 insertions(+), 30 deletions(-)

diff --git a/modules/signals/rxjs-interop/spec/rx-method.spec.ts b/modules/signals/rxjs-interop/spec/rx-method.spec.ts
index 419994dfc0..6b177975ee 100644
--- a/modules/signals/rxjs-interop/spec/rx-method.spec.ts
+++ b/modules/signals/rxjs-interop/spec/rx-method.spec.ts
@@ -251,18 +251,26 @@ describe('rxMethod', () => {
     });
   });
 
-  describe('Signals and effect injector', () => {
+  /**
+   * This test suite verifies that the internal effect of
+   * an RxMethod instance is executed with the correct injector
+   * and is destroyed at the specified time.
+   *
+   * Since we cannot directly observe the destruction of the effect from the outside,
+   * we test it indirectly.
+   *
+   * Components use the globalSignal from GlobalService and pass it
+   * to the `log` method. If the component is destroyed but a subsequent
+   * Signal change still increases the `globalSignalChangerCounter`,
+   * it indicates that the internal effect is still active.
+   */
+  describe('Internal effect for Signal tracking', () => {
     @Injectable({ providedIn: 'root' })
     class GlobalService {
-      rxMethodStatus = 'none';
-      log = rxMethod<string>(
-        pipe(
-          tap({
-            next: () => (this.rxMethodStatus = 'started'),
-            finalize: () => (this.rxMethodStatus = 'destroyed'),
-          })
-        )
-      );
+      globalSignal = signal(1);
+      globalSignalChangeCounter = 0;
+
+      log = rxMethod<number>(pipe(tap(() => this.globalSignalChangeCounter++)));
     }
 
     @Component({
@@ -289,6 +297,34 @@ describe('rxMethod', () => {
       return TestBed.inject(GlobalService);
     }
 
+    it('it tracks the Signal when component is active', async () => {
+      @Component({
+        selector: 'app-with-store',
+        template: ``,
+        standalone: true,
+      })
+      class WithStoreComponent {
+        store = inject(GlobalService);
+
+        constructor() {
+          this.store.log(this.store.globalSignal);
+        }
+      }
+
+      const globalService = setup(WithStoreComponent);
+
+      await RouterTestingHarness.create('/with-store');
+      expect(globalService.globalSignalChangeCounter).toBe(1);
+
+      globalService.globalSignal.update((value) => value + 1);
+      TestBed.flushEffects();
+      expect(globalService.globalSignalChangeCounter).toBe(2);
+
+      globalService.globalSignal.update((value) => value + 1);
+      TestBed.flushEffects();
+      expect(globalService.globalSignalChangeCounter).toBe(3);
+    });
+
     it('destroys with component injector when rxMethod is in root and RxMethod in component', async () => {
       @Component({
         selector: 'app-with-store',
@@ -299,16 +335,20 @@ describe('rxMethod', () => {
         store = inject(GlobalService);
 
         constructor() {
-          this.store.log(signal('test'));
+          this.store.log(this.store.globalSignal);
         }
       }
 
       const globalService = setup(WithStoreComponent);
 
       const harness = await RouterTestingHarness.create('/with-store');
-      expect(globalService.rxMethodStatus).toBe('started');
+
+      // effect is destroyed → Signal is not tracked anymore
       await harness.navigateByUrl('/without-store');
-      expect(globalService.rxMethodStatus).toBe('destroyed');
+      globalService.globalSignal.update((value) => value + 1);
+      TestBed.flushEffects();
+
+      expect(globalService.globalSignalChangeCounter).toBe(1);
     });
 
     it("falls back to rxMethod's injector when RxMethod's call is outside of injection context", async () => {
@@ -321,16 +361,20 @@ describe('rxMethod', () => {
         store = inject(GlobalService);
 
         ngOnInit() {
-          this.store.log(signal('test'));
+          this.store.log(this.store.globalSignal);
         }
       }
 
       const globalService = setup(WithStoreComponent);
 
       const harness = await RouterTestingHarness.create('/with-store');
-      expect(globalService.rxMethodStatus).toBe('started');
+
+      // Signal is still tracked because RxMethod injector is used
       await harness.navigateByUrl('/without-store');
-      expect(globalService.rxMethodStatus).toBe('started');
+      globalService.globalSignal.update((value) => value + 1);
+      TestBed.flushEffects();
+
+      expect(globalService.globalSignalChangeCounter).toBe(2);
     });
 
     it('provides the injector for RxMethod on call', async () => {
@@ -344,16 +388,20 @@ describe('rxMethod', () => {
         injector = inject(Injector);
 
         ngOnInit() {
-          this.store.log(signal('test'), this.injector);
+          this.store.log(this.store.globalSignal, this.injector);
         }
       }
 
       const globalService = setup(WithStoreComponent);
 
       const harness = await RouterTestingHarness.create('/with-store');
-      expect(globalService.rxMethodStatus).toBe('started');
+
+      // effect is destroyed → Signal is not tracked anymore
       await harness.navigateByUrl('/without-store');
-      expect(globalService.rxMethodStatus).toBe('destroyed');
+      globalService.globalSignal.update((value) => value + 1);
+      TestBed.flushEffects();
+
+      expect(globalService.globalSignalChangeCounter).toBe(1);
     });
   });
 });
diff --git a/modules/signals/rxjs-interop/src/rx-method.ts b/modules/signals/rxjs-interop/src/rx-method.ts
index d93e57d213..e6e5e582bd 100644
--- a/modules/signals/rxjs-interop/src/rx-method.ts
+++ b/modules/signals/rxjs-interop/src/rx-method.ts
@@ -25,8 +25,11 @@ export function rxMethod<Input>(
   }
 
   const injector = config?.injector ?? inject(Injector);
+  const destroyRef = injector.get(DestroyRef);
   const source$ = new Subject<Input>();
+
   const sourceSub = generator(source$).subscribe();
+  destroyRef.onDestroy(() => sourceSub.unsubscribe());
 
   const rxMethodFn = (
     input: Input | Signal<Input> | Observable<Input>,
@@ -43,14 +46,7 @@ export function rxMethod<Input>(
         },
         { injector: instanceInjector }
       );
-
-      instanceInjector.get(DestroyRef).onDestroy(() => {
-        sourceSub.unsubscribe();
-      });
-
-      const instanceSub = {
-        unsubscribe: () => watcher.destroy(),
-      };
+      const instanceSub = { unsubscribe: () => watcher.destroy() };
       sourceSub.add(instanceSub);
 
       return instanceSub;
@@ -60,9 +56,6 @@ export function rxMethod<Input>(
       const instanceSub = input.subscribe((value) => source$.next(value));
       sourceSub.add(instanceSub);
 
-      const destroyRef = injector.get(DestroyRef);
-      destroyRef.onDestroy(() => sourceSub.unsubscribe());
-
       return instanceSub;
     }
 

From 9637784519724158247a4eb653a705f65654ad17 Mon Sep 17 00:00:00 2001
From: Rainer Hahnekamp <rainer.hahnekamp@gmail.com>
Date: Tue, 24 Sep 2024 19:37:21 +0200
Subject: [PATCH 4/9] Update modules/signals/rxjs-interop/src/rx-method.ts
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Co-authored-by: Marko Stanimirović <markostanimirovic95@gmail.com>
---
 modules/signals/rxjs-interop/src/rx-method.ts | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/modules/signals/rxjs-interop/src/rx-method.ts b/modules/signals/rxjs-interop/src/rx-method.ts
index d93e57d213..244c6a6299 100644
--- a/modules/signals/rxjs-interop/src/rx-method.ts
+++ b/modules/signals/rxjs-interop/src/rx-method.ts
@@ -12,7 +12,7 @@ import { isObservable, noop, Observable, Subject, Unsubscribable } from 'rxjs';
 
 type RxMethod<Input> = ((
   input: Input | Signal<Input> | Observable<Input>,
-  injector?: Injector
+  config?: { injector?: Injector }
 ) => Unsubscribable) &
   Unsubscribable;
 

From db893a02218f479a9e5776953bfbc55108329325 Mon Sep 17 00:00:00 2001
From: Rainer Hahnekamp <rainer.hahnekamp@gmail.com>
Date: Tue, 24 Sep 2024 22:02:31 +0200
Subject: [PATCH 5/9] refactor: use config object customInjector

---
 modules/signals/rxjs-interop/spec/rx-method.spec.ts | 2 +-
 modules/signals/rxjs-interop/src/rx-method.ts       | 4 +++-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/modules/signals/rxjs-interop/spec/rx-method.spec.ts b/modules/signals/rxjs-interop/spec/rx-method.spec.ts
index 6b177975ee..833d611b23 100644
--- a/modules/signals/rxjs-interop/spec/rx-method.spec.ts
+++ b/modules/signals/rxjs-interop/spec/rx-method.spec.ts
@@ -388,7 +388,7 @@ describe('rxMethod', () => {
         injector = inject(Injector);
 
         ngOnInit() {
-          this.store.log(this.store.globalSignal, this.injector);
+          this.store.log(this.store.globalSignal, { injector: this.injector });
         }
       }
 
diff --git a/modules/signals/rxjs-interop/src/rx-method.ts b/modules/signals/rxjs-interop/src/rx-method.ts
index f3637a1946..bf65d1b2e3 100644
--- a/modules/signals/rxjs-interop/src/rx-method.ts
+++ b/modules/signals/rxjs-interop/src/rx-method.ts
@@ -4,6 +4,7 @@ import {
   effect,
   inject,
   Injector,
+  input,
   isSignal,
   Signal,
   untracked,
@@ -33,10 +34,11 @@ export function rxMethod<Input>(
 
   const rxMethodFn = (
     input: Input | Signal<Input> | Observable<Input>,
-    customInjector?: Injector
+    config?: { injector?: Injector }
   ) => {
     if (isSignal(input)) {
       const callerInjector = getCallerInjectorIfAvailable();
+      const customInjector = config?.injector;
       const instanceInjector = customInjector ?? callerInjector ?? injector;
 
       const watcher = effect(

From 600d575c6b497d82437da2c9ee5a08613a1200dd Mon Sep 17 00:00:00 2001
From: Rainer Hahnekamp <rainer.hahnekamp@gmail.com>
Date: Tue, 24 Sep 2024 22:03:41 +0200
Subject: [PATCH 6/9] refactor: remove unused `input` import

---
 modules/signals/rxjs-interop/src/rx-method.ts | 1 -
 1 file changed, 1 deletion(-)

diff --git a/modules/signals/rxjs-interop/src/rx-method.ts b/modules/signals/rxjs-interop/src/rx-method.ts
index bf65d1b2e3..637276efd7 100644
--- a/modules/signals/rxjs-interop/src/rx-method.ts
+++ b/modules/signals/rxjs-interop/src/rx-method.ts
@@ -4,7 +4,6 @@ import {
   effect,
   inject,
   Injector,
-  input,
   isSignal,
   Signal,
   untracked,

From 2deed392d3bd81b5eff05c2177f9b85a69274992 Mon Sep 17 00:00:00 2001
From: Rainer Hahnekamp <rainer.hahnekamp@gmail.com>
Date: Wed, 25 Sep 2024 19:59:22 +0200
Subject: [PATCH 7/9] Update modules/signals/rxjs-interop/src/rx-method.ts

Co-authored-by: Tim Deschryver <28659384+timdeschryver@users.noreply.github.com>
---
 modules/signals/rxjs-interop/src/rx-method.ts | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/modules/signals/rxjs-interop/src/rx-method.ts b/modules/signals/rxjs-interop/src/rx-method.ts
index 637276efd7..574c9f5fb1 100644
--- a/modules/signals/rxjs-interop/src/rx-method.ts
+++ b/modules/signals/rxjs-interop/src/rx-method.ts
@@ -36,9 +36,7 @@ export function rxMethod<Input>(
     config?: { injector?: Injector }
   ) => {
     if (isSignal(input)) {
-      const callerInjector = getCallerInjectorIfAvailable();
-      const customInjector = config?.injector;
-      const instanceInjector = customInjector ?? callerInjector ?? injector;
+      const instanceInjector = config?.injector ?? getCallerInjectorIfAvailable() ?? injector;
 
       const watcher = effect(
         () => {

From b3fcf82b68ccf25d2b7992cf105bc1b209954d4c Mon Sep 17 00:00:00 2001
From: markostanimirovic <markostanimirovic95@gmail.com>
Date: Sat, 28 Sep 2024 03:04:49 +0200
Subject: [PATCH 8/9] fix(signals): do not listen to observable changes on
 instance injector destroy

---
 .../rxjs-interop/spec/rx-method.spec.ts       | 169 ++++++++++++------
 modules/signals/rxjs-interop/src/rx-method.ts |  37 ++--
 2 files changed, 133 insertions(+), 73 deletions(-)

diff --git a/modules/signals/rxjs-interop/spec/rx-method.spec.ts b/modules/signals/rxjs-interop/spec/rx-method.spec.ts
index 833d611b23..5dcef598f0 100644
--- a/modules/signals/rxjs-interop/spec/rx-method.spec.ts
+++ b/modules/signals/rxjs-interop/spec/rx-method.spec.ts
@@ -9,12 +9,12 @@ import {
   signal,
 } from '@angular/core';
 import { TestBed } from '@angular/core/testing';
-import { BehaviorSubject, finalize, pipe, Subject, tap } from 'rxjs';
-import { rxMethod } from '../src';
-import { createLocalService } from '../../spec/helpers';
-import { provideRouter } from '@angular/router';
 import { provideLocationMocks } from '@angular/common/testing';
+import { provideRouter } from '@angular/router';
 import { RouterTestingHarness } from '@angular/router/testing';
+import { BehaviorSubject, pipe, Subject, tap } from 'rxjs';
+import { rxMethod } from '../src';
+import { createLocalService } from '../../spec/helpers';
 
 describe('rxMethod', () => {
   it('runs with a value', () => {
@@ -239,43 +239,44 @@ describe('rxMethod', () => {
     expect(counter()).toBe(4);
   });
 
-  it('completes on manual destroy with Signals', () => {
-    TestBed.runInInjectionContext(() => {
-      let completed = false;
-      const counter = signal(1);
-      const fn = rxMethod<number>(finalize(() => (completed = true)));
-      TestBed.flushEffects();
-      fn(counter);
-      fn.unsubscribe();
-      expect(completed).toBe(true);
-    });
-  });
-
   /**
-   * This test suite verifies that the internal effect of
-   * an RxMethod instance is executed with the correct injector
-   * and is destroyed at the specified time.
-   *
-   * Since we cannot directly observe the destruction of the effect from the outside,
-   * we test it indirectly.
+   * This test suite verifies that a signal or observable passed to a reactive
+   * method that is initialized at the ancestor injector level is tracked within
+   * the correct injection context and untracked at the specified time.
    *
-   * Components use the globalSignal from GlobalService and pass it
-   * to the `log` method. If the component is destroyed but a subsequent
-   * Signal change still increases the `globalSignalChangerCounter`,
-   * it indicates that the internal effect is still active.
+   * Components use `globalSignal` or `globalObservable` from `GlobalService`
+   * and pass it to the reactive method. If the component is destroyed but
+   * signal or observable change still increases the corresponding counter,
+   * the internal effect or subscription is still active.
    */
-  describe('Internal effect for Signal tracking', () => {
+  describe('with instance injector', () => {
     @Injectable({ providedIn: 'root' })
     class GlobalService {
-      globalSignal = signal(1);
+      readonly globalSignal = signal(1);
+      readonly globalObservable = new BehaviorSubject(1);
+
       globalSignalChangeCounter = 0;
+      globalObservableChangeCounter = 0;
+
+      readonly signalMethod = rxMethod<number>(
+        tap(() => this.globalSignalChangeCounter++)
+      );
+      readonly observableMethod = rxMethod<number>(
+        tap(() => this.globalObservableChangeCounter++)
+      );
 
-      log = rxMethod<number>(pipe(tap(() => this.globalSignalChangeCounter++)));
+      incrementSignal(): void {
+        this.globalSignal.update((value) => value + 1);
+      }
+
+      incrementObservable(): void {
+        this.globalObservable.next(this.globalObservable.value + 1);
+      }
     }
 
     @Component({
-      selector: `app-storeless`,
-      template: ``,
+      selector: 'app-without-store',
+      template: '',
       standalone: true,
     })
     class WithoutStoreComponent {}
@@ -297,90 +298,107 @@ describe('rxMethod', () => {
       return TestBed.inject(GlobalService);
     }
 
-    it('it tracks the Signal when component is active', async () => {
+    it('tracks a signal until the component is destroyed', async () => {
       @Component({
         selector: 'app-with-store',
-        template: ``,
+        template: '',
         standalone: true,
       })
       class WithStoreComponent {
         store = inject(GlobalService);
 
         constructor() {
-          this.store.log(this.store.globalSignal);
+          this.store.signalMethod(this.store.globalSignal);
         }
       }
 
       const globalService = setup(WithStoreComponent);
+      const harness = await RouterTestingHarness.create('/with-store');
 
-      await RouterTestingHarness.create('/with-store');
       expect(globalService.globalSignalChangeCounter).toBe(1);
 
-      globalService.globalSignal.update((value) => value + 1);
+      globalService.incrementSignal();
       TestBed.flushEffects();
       expect(globalService.globalSignalChangeCounter).toBe(2);
 
-      globalService.globalSignal.update((value) => value + 1);
+      globalService.incrementSignal();
       TestBed.flushEffects();
       expect(globalService.globalSignalChangeCounter).toBe(3);
+
+      await harness.navigateByUrl('/without-store');
+      globalService.incrementSignal();
+      TestBed.flushEffects();
+
+      expect(globalService.globalSignalChangeCounter).toBe(3);
     });
 
-    it('destroys with component injector when rxMethod is in root and RxMethod in component', async () => {
+    it('tracks an observable until the component is destroyed', async () => {
       @Component({
         selector: 'app-with-store',
-        template: ``,
+        template: '',
         standalone: true,
       })
       class WithStoreComponent {
         store = inject(GlobalService);
 
         constructor() {
-          this.store.log(this.store.globalSignal);
+          this.store.observableMethod(this.store.globalObservable);
         }
       }
 
       const globalService = setup(WithStoreComponent);
-
       const harness = await RouterTestingHarness.create('/with-store');
 
-      // effect is destroyed → Signal is not tracked anymore
+      expect(globalService.globalObservableChangeCounter).toBe(1);
+
+      globalService.incrementObservable();
+      expect(globalService.globalObservableChangeCounter).toBe(2);
+
+      globalService.incrementObservable();
+      expect(globalService.globalObservableChangeCounter).toBe(3);
+
       await harness.navigateByUrl('/without-store');
-      globalService.globalSignal.update((value) => value + 1);
-      TestBed.flushEffects();
+      globalService.incrementObservable();
 
-      expect(globalService.globalSignalChangeCounter).toBe(1);
+      expect(globalService.globalObservableChangeCounter).toBe(3);
     });
 
-    it("falls back to rxMethod's injector when RxMethod's call is outside of injection context", async () => {
+    it('tracks a signal until the provided injector is destroyed', async () => {
       @Component({
-        selector: `app-store`,
-        template: ``,
+        selector: 'app-with-store',
+        template: '',
         standalone: true,
       })
       class WithStoreComponent implements OnInit {
         store = inject(GlobalService);
+        injector = inject(Injector);
 
         ngOnInit() {
-          this.store.log(this.store.globalSignal);
+          this.store.signalMethod(this.store.globalSignal, {
+            injector: this.injector,
+          });
         }
       }
 
       const globalService = setup(WithStoreComponent);
-
       const harness = await RouterTestingHarness.create('/with-store');
 
-      // Signal is still tracked because RxMethod injector is used
+      globalService.incrementSignal();
+      TestBed.flushEffects();
+
+      expect(globalService.globalSignalChangeCounter).toBe(2);
+
       await harness.navigateByUrl('/without-store');
-      globalService.globalSignal.update((value) => value + 1);
+      globalService.incrementSignal();
       TestBed.flushEffects();
 
       expect(globalService.globalSignalChangeCounter).toBe(2);
     });
 
-    it('provides the injector for RxMethod on call', async () => {
+    it('tracks an observable until the provided injector is destroyed', async () => {
       @Component({
-        selector: `app-store`,
-        template: ``,
+        selector: 'app-with-store',
+        template: '',
         standalone: true,
       })
       class WithStoreComponent implements OnInit {
@@ -388,20 +406,53 @@ describe('rxMethod', () => {
         injector = inject(Injector);
 
         ngOnInit() {
-          this.store.log(this.store.globalSignal, { injector: this.injector });
+          this.store.observableMethod(this.store.globalObservable, {
+            injector: this.injector,
+          });
         }
       }
 
       const globalService = setup(WithStoreComponent);
+      const harness = await RouterTestingHarness.create('/with-store');
+
+      globalService.incrementObservable();
+
+      expect(globalService.globalObservableChangeCounter).toBe(2);
+
+      await harness.navigateByUrl('/without-store');
+      globalService.incrementObservable();
+
+      expect(globalService.globalObservableChangeCounter).toBe(2);
+    });
 
+    it('falls back to source injector when reactive method is called is outside of injection context', async () => {
+      @Component({
+        selector: 'app-with-store',
+        template: '',
+        standalone: true,
+      })
+      class WithStoreComponent implements OnInit {
+        store = inject(GlobalService);
+
+        ngOnInit() {
+          this.store.signalMethod(this.store.globalSignal);
+          this.store.observableMethod(this.store.globalObservable);
+        }
+      }
+
+      const globalService = setup(WithStoreComponent);
       const harness = await RouterTestingHarness.create('/with-store');
 
-      // effect is destroyed → Signal is not tracked anymore
+      expect(globalService.globalSignalChangeCounter).toBe(1);
+      expect(globalService.globalObservableChangeCounter).toBe(1);
+
       await harness.navigateByUrl('/without-store');
-      globalService.globalSignal.update((value) => value + 1);
+      globalService.incrementSignal();
       TestBed.flushEffects();
+      globalService.incrementObservable();
 
-      expect(globalService.globalSignalChangeCounter).toBe(1);
+      expect(globalService.globalSignalChangeCounter).toBe(2);
+      expect(globalService.globalObservableChangeCounter).toBe(2);
     });
   });
 });
diff --git a/modules/signals/rxjs-interop/src/rx-method.ts b/modules/signals/rxjs-interop/src/rx-method.ts
index 574c9f5fb1..5d93ff877f 100644
--- a/modules/signals/rxjs-interop/src/rx-method.ts
+++ b/modules/signals/rxjs-interop/src/rx-method.ts
@@ -24,20 +24,24 @@ export function rxMethod<Input>(
     assertInInjectionContext(rxMethod);
   }
 
-  const injector = config?.injector ?? inject(Injector);
-  const destroyRef = injector.get(DestroyRef);
+  const sourceInjector = config?.injector ?? inject(Injector);
   const source$ = new Subject<Input>();
-
   const sourceSub = generator(source$).subscribe();
-  destroyRef.onDestroy(() => sourceSub.unsubscribe());
+  sourceInjector.get(DestroyRef).onDestroy(() => sourceSub.unsubscribe());
 
   const rxMethodFn = (
     input: Input | Signal<Input> | Observable<Input>,
     config?: { injector?: Injector }
   ) => {
-    if (isSignal(input)) {
-      const instanceInjector = config?.injector ?? getCallerInjectorIfAvailable() ?? injector;
+    if (isStatic(input)) {
+      source$.next(input);
+      return { unsubscribe: noop };
+    }
 
+    const instanceInjector =
+      config?.injector ?? getCallerInjector() ?? sourceInjector;
+
+    if (isSignal(input)) {
       const watcher = effect(
         () => {
           const value = input();
@@ -51,25 +55,30 @@ export function rxMethod<Input>(
       return instanceSub;
     }
 
-    if (isObservable(input)) {
-      const instanceSub = input.subscribe((value) => source$.next(value));
-      sourceSub.add(instanceSub);
+    const instanceSub = input.subscribe((value) => source$.next(value));
+    sourceSub.add(instanceSub);
 
-      return instanceSub;
+    if (instanceInjector !== sourceInjector) {
+      instanceInjector
+        .get(DestroyRef)
+        .onDestroy(() => instanceSub.unsubscribe());
     }
 
-    source$.next(input);
-    return { unsubscribe: noop };
+    return instanceSub;
   };
   rxMethodFn.unsubscribe = sourceSub.unsubscribe.bind(sourceSub);
 
   return rxMethodFn;
 }
 
-function getCallerInjectorIfAvailable(): Injector | null {
+function isStatic<T>(value: T | Signal<T> | Observable<T>): value is T {
+  return !isSignal(value) && !isObservable(value);
+}
+
+function getCallerInjector(): Injector | null {
   try {
     return inject(Injector);
-  } catch (e) {
+  } catch {
     return null;
   }
 }

From 1ffc0c76e2d0732096ee74a92566e138ef44927d Mon Sep 17 00:00:00 2001
From: Tim Deschryver <28659384+timdeschryver@users.noreply.github.com>
Date: Sat, 28 Sep 2024 13:13:10 +0200
Subject: [PATCH 9/9] Update
 modules/signals/rxjs-interop/spec/rx-method.spec.ts

---
 modules/signals/rxjs-interop/spec/rx-method.spec.ts | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/modules/signals/rxjs-interop/spec/rx-method.spec.ts b/modules/signals/rxjs-interop/spec/rx-method.spec.ts
index 5dcef598f0..f3f83fc7c6 100644
--- a/modules/signals/rxjs-interop/spec/rx-method.spec.ts
+++ b/modules/signals/rxjs-interop/spec/rx-method.spec.ts
@@ -425,7 +425,7 @@ describe('rxMethod', () => {
       expect(globalService.globalObservableChangeCounter).toBe(2);
     });
 
-    it('falls back to source injector when reactive method is called is outside of injection context', async () => {
+    it('falls back to source injector when reactive method is called outside of the injection context', async () => {
       @Component({
         selector: 'app-with-store',
         template: '',