cancellableTimeout() is compatible with DestroyRef #10746

projects/natural/src/lib/classes/rxjs.spec.ts

@@ -1,6 +1,21 @@
 import {cancellableTimeout} from './rxjs';
 import {ReplaySubject, Subject} from 'rxjs';
 import {fakeAsync, tick} from '@angular/core/testing';
+import {DestroyRef} from '@angular/core';
+
+class TestDestroyRef extends DestroyRef {
+    private callback: (() => void) | null = null;
+
+    public override onDestroy(callback: () => void): () => void {
+        this.callback = callback;
+        return () => undefined;
+    }
+
+    public destroy(): void {
+        this.callback?.();
+        this.callback = null;
+    }
+}
 
 describe('cancellableTimeout', () => {
     const observer = {
@@ -64,4 +79,50 @@ describe('cancellableTimeout', () => {
         expect(count).withContext('already completed, nothing change').toBe(0);
         expect(completed).toBe(true);
     }));
+
+    it('run the callback exactly once with DestroyRef', fakeAsync(() => {
+        const canceller = new TestDestroyRef();
+        const timeout = cancellableTimeout(canceller);
+
+        expect(count).withContext('nothing happened yet').toBe(0);
+        expect(completed).toBe(false);
+
+        tick();
+        expect(count).withContext('still nothing happened because no subscriber').toBe(0);
+        expect(completed).toBe(false);
+
+        timeout.subscribe(observer);
+        expect(count).withContext('still nothing happened because time did not pass').toBe(0);
+        expect(completed).toBe(false);
+
+        tick();
+        expect(count).withContext('callback called exactly once').toBe(1);
+        expect(completed).toBe(true);
+
+        canceller.destroy();
+        tick();
+        expect(count).withContext('already completed, nothing change').toBe(1);
+        expect(completed).toBe(true);
+    }));
+
+    it('never run the callback if cancelled with DestroyRef', fakeAsync(() => {
+        const canceller = new TestDestroyRef();
+        const timeout = cancellableTimeout(canceller);
+
+        expect(count).withContext('nothing happened yet').toBe(0);
+        expect(completed).toBe(false);
+
+        tick();
+        expect(count).withContext('still nothing happened because no subscriber').toBe(0);
+        expect(completed).toBe(false);
+
+        timeout.subscribe(observer);
+        canceller.destroy();
+        expect(count).withContext('still nothing happened because cancelled').toBe(0);
+        expect(completed).toBe(true);
+
+        tick();
+        expect(count).withContext('already completed, nothing change').toBe(0);
+        expect(completed).toBe(true);
+    }));
 });

projects/natural/src/lib/classes/rxjs.ts

@@ -1,4 +1,6 @@
 import {map, MonoTypeOperatorFunction, Observable, take, takeUntil, tap, timer} from 'rxjs';
+import {DestroyRef} from '@angular/core';
+import {takeUntilDestroyed} from '@angular/core/rxjs-interop';
 
 /**
  * Behave like setTimeout(), but with a mandatory cancel mechanism.
@@ -10,10 +12,16 @@ import {map, MonoTypeOperatorFunction, Observable, take, takeUntil, tap, timer}
  * Typical usage in a component would be:
  *
  * ```ts
+ * cancellableTimeout(inject(DestroyRef)).subscribe(myCallback);
+ * ```
+ *
+ * or
+ *
+ * ```ts
  * cancellableTimeout(this.ngUnsubscribe).subscribe(myCallback);
  * ```
  *
- * Instead of the more error prone:
+ * Instead of the more error-prone:
  *
  * ```ts
  * public foo(): void {
@@ -28,13 +36,11 @@ import {map, MonoTypeOperatorFunction, Observable, take, takeUntil, tap, timer}
  * }
  * ```
  */
-export function cancellableTimeout(canceller: Observable<unknown>, milliSeconds = 0): Observable<void> {
+export function cancellableTimeout(canceller: Observable<unknown> | DestroyRef, milliSeconds = 0): Observable<void> {
     return timer(milliSeconds).pipe(
         take(1),
-        takeUntil(canceller),
-        map(() => {
-            return;
-        }),
+        canceller instanceof DestroyRef ? takeUntilDestroyed(canceller) : takeUntil(canceller),
+        map(() => undefined),
     );
 }
 

projects/natural/src/lib/modules/columns-picker/columns-picker.component.ts

@@ -1,7 +1,7 @@
-import {Component, EventEmitter, Input, OnChanges, OnDestroy, Output, SimpleChanges} from '@angular/core';
+import {Component, DestroyRef, EventEmitter, inject, Input, OnChanges, Output, SimpleChanges} from '@angular/core';
 import {AvailableColumn, Button} from './types';
 import {cancellableTimeout} from '../../classes/rxjs';
-import {map, Subject} from 'rxjs';
+import {map} from 'rxjs';
 import {ThemePalette} from '@angular/material/core';
 import {BreakpointObserver, Breakpoints} from '@angular/cdk/layout';
 import {FormsModule} from '@angular/forms';
@@ -29,7 +29,8 @@ import {CommonModule} from '@angular/common';
         FormsModule,
     ],
 })
-export class NaturalColumnsPickerComponent implements OnChanges, OnDestroy {
+export class NaturalColumnsPickerComponent implements OnChanges {
+    private readonly destroyRef = inject(DestroyRef);
     private _selections?: string[];
     private _availableColumns: Required<AvailableColumn>[] = [];
 
@@ -82,8 +83,6 @@ export class NaturalColumnsPickerComponent implements OnChanges, OnDestroy {
      */
     public displayedColumns: Required<AvailableColumn>[] = [];
 
-    private readonly ngUnsubscribe = new Subject<void>();
-
     public readonly isMobile = this.breakpointObserver.observe(Breakpoints.XSmall).pipe(map(result => result.matches));
 
     public constructor(private readonly breakpointObserver: BreakpointObserver) {}
@@ -104,7 +103,7 @@ export class NaturalColumnsPickerComponent implements OnChanges, OnDestroy {
 
     public ngOnChanges(changes: SimpleChanges): void {
         // Unfortunately need a timeout to avoid an ExpressionChangedAfterItHasBeenCheckedError on /state/4989/process
-        cancellableTimeout(this.ngUnsubscribe).subscribe(() => {
+        cancellableTimeout(this.destroyRef).subscribe(() => {
             if (changes.availableColumns) {
                 this.initColumns();
                 this.updateColumns();
@@ -114,11 +113,6 @@ export class NaturalColumnsPickerComponent implements OnChanges, OnDestroy {
         });
     }
 
-    public ngOnDestroy(): void {
-        this.ngUnsubscribe.next(); // unsubscribe everybody
-        this.ngUnsubscribe.complete(); // complete the stream, because we will never emit again
-    }
-
     public defaultTrue(value: boolean | undefined): boolean {
         return value ?? true;
     }