fix(core): takeUntilDestroyed completes immediately if DestroyRef already destroyed (#61847)

Adds fix directly for `takeUntilDestroyed` to unsubscribe when already
destroyed instead of putting
synchronous behavior on `DestroyRef.onDestroyed` callback as in #58008

fixes #54527

PR Close #61847
This commit is contained in:
Andrew Scott 2025-05-27 10:38:29 -07:00 committed by kirjs
parent dcfbe6c811
commit 2bd3a43028
4 changed files with 62 additions and 5 deletions

View file

@ -26,8 +26,12 @@ export function takeUntilDestroyed<T>(destroyRef?: DestroyRef): MonoTypeOperator
destroyRef = inject(DestroyRef);
}
const destroyed$ = new Observable<void>((observer) => {
const unregisterFn = destroyRef!.onDestroy(observer.next.bind(observer));
const destroyed$ = new Observable<void>((subscriber) => {
if ((destroyRef as unknown as {destroyed: boolean}).destroyed) {
subscriber.next();
return;
}
const unregisterFn = destroyRef!.onDestroy(subscriber.next.bind(subscriber));
return unregisterFn;
});

View file

@ -6,10 +6,18 @@
* found in the LICENSE file at https://angular.dev/license
*/
import {DestroyRef, EnvironmentInjector, Injector, runInInjectionContext} from '../../src/core';
import {BehaviorSubject} from 'rxjs';
import {
Component,
DestroyRef,
EnvironmentInjector,
inject,
Injector,
runInInjectionContext,
} from '../../src/core';
import {BehaviorSubject, finalize} from 'rxjs';
import {takeUntilDestroyed} from '../src/take_until_destroyed';
import {TestBed} from '@angular/core/testing';
describe('takeUntilDestroyed', () => {
it('should complete an observable when the current context is destroyed', () => {
@ -76,4 +84,39 @@ describe('takeUntilDestroyed', () => {
expect(unregisterFn).toHaveBeenCalled();
});
// https://github.com/angular/angular/issues/54527
it('should unsubscribe after the current context has already been destroyed', async () => {
const recorder: any[] = [];
// Note that we need a "real" view for this test because, in other cases,
// `DestroyRef` would resolve to the root injector rather than to the
// `NodeInjectorDestroyRef`, where `lView` is used.
@Component({template: ''})
class TestComponent {
destroyRef = inject(DestroyRef);
source$ = new BehaviorSubject(0);
ngOnDestroy(): void {
Promise.resolve().then(() => {
this.source$
.pipe(
takeUntilDestroyed(this.destroyRef),
finalize(() => recorder.push('finalize()')),
)
.subscribe((value) => recorder.push(value));
});
}
}
const fixture = TestBed.createComponent(TestComponent);
fixture.destroy();
// Wait until the `ngOnDestroy` microtask is run.
await Promise.resolve();
// Ensure the `value` is not recorded, but unsubscribed immediately.
expect(recorder).toEqual(['finalize()']);
});
});

View file

@ -175,6 +175,9 @@ export abstract class EnvironmentInjector implements Injector {
abstract destroy(): void;
/** @internal */
abstract get destroyed(): boolean;
/**
* @internal
*/
@ -199,7 +202,7 @@ export class R3Injector extends EnvironmentInjector implements PrimitivesInjecto
/**
* Flag indicating that this injector was previously destroyed.
*/
get destroyed(): boolean {
override get destroyed(): boolean {
return this._destroyed;
}
private _destroyed = false;

View file

@ -46,6 +46,9 @@ export abstract class DestroyRef {
*/
abstract onDestroy(callback: () => void): () => void;
/** @internal */
abstract get destroyed(): boolean;
/**
* @internal
* @nocollapse
@ -64,6 +67,10 @@ export class NodeInjectorDestroyRef extends DestroyRef {
super();
}
override get destroyed() {
return isDestroyed(this._lView);
}
override onDestroy(callback: () => void): () => void {
const lView = this._lView;