fix(core): ensure takeUntilDestroyed unregisters onDestroy listener on unsubscribe (#49901)

skrtheboss · thePunderWoman · commit c029c678d958 · 2023-04-19T19:55:09.000Z
The takeUntilDestroyed must always remove the onDestroy listener, in the teardown logic. PR Close #49901
diff --git a/packages/core/rxjs-interop/src/take_until_destroyed.ts b/packages/core/rxjs-interop/src/take_until_destroyed.ts
@@ -27,7 +27,8 @@ export function takeUntilDestroyed<T>(destroyRef?: DestroyRef): MonoTypeOperator
   }
 
   const destroyed$ = new Observable<void>(observer => {
-    destroyRef!.onDestroy(observer.next.bind(observer));
+    const unregisterFn = destroyRef!.onDestroy(observer.next.bind(observer));
+    return unregisterFn;
   });
 
   return <T>(source: Observable<T>) => {
diff --git a/packages/core/rxjs-interop/test/take_until_destroyed_spec.ts b/packages/core/rxjs-interop/test/take_until_destroyed_spec.ts
@@ -63,4 +63,17 @@ describe('takeUntilDestroyed', () => {
     source$.next(2);
     expect(last).toBe(1);
   });
+
+  it('should unregister listener if observable is unsubscribed', () => {
+    const injector = Injector.create({providers: []}) as EnvironmentInjector;
+    const destroyRef = injector.get(DestroyRef);
+    const unregisterFn = jasmine.createSpy();
+    spyOn(destroyRef, 'onDestroy').and.returnValue(unregisterFn);
+
+    const subscription = new BehaviorSubject(0).pipe(takeUntilDestroyed(destroyRef)).subscribe();
+
+    subscription.unsubscribe();
+
+    expect(unregisterFn).toHaveBeenCalled();
+  });
 });

Original file line number	Diff line number	Diff line change
`@@ -27,7 +27,8 @@ export function takeUntilDestroyed<T>(destroyRef?: DestroyRef): MonoTypeOperator`
`27`	`27`	`}`
`28`	`28`
`29`	`29`	`const destroyed$ = new Observable<void>(observer => {`
`30`		`- destroyRef!.onDestroy(observer.next.bind(observer));`
	`30`	`+ const unregisterFn = destroyRef!.onDestroy(observer.next.bind(observer));`
	`31`	`+ return unregisterFn;`
`31`	`32`	`});`
`32`	`33`
`33`	`34`	`return <T>(source: Observable<T>) => {`