Revert "fix(core): call DestroyRef on destroy callback if view is destroyed (angular#58008)" (angular#61625)

This reverts commit 5f7f046.

PR Close angular#61625

Author: JeanMeche

packages/core/rxjs-interop/test/take_until_destroyed_spec.ts

@@ -6,17 +6,8 @@
  * found in the LICENSE file at https://angular.dev/license
  */
 
-import {
-  Component,
-  DestroyRef,
-  EnvironmentInjector,
-  inject,
-  Injector,
-  OnDestroy,
-  runInInjectionContext,
-} from '../../src/core';
-import {TestBed} from '../../testing';
-import {BehaviorSubject, finalize} from 'rxjs';
+import {DestroyRef, EnvironmentInjector, Injector, runInInjectionContext} from '../../src/core';
+import {BehaviorSubject} from 'rxjs';
 
 import {takeUntilDestroyed} from '../src/take_until_destroyed';
 
@@ -85,39 +76,4 @@ 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 implements OnDestroy {
-      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()']);
-  });
 });

packages/core/src/linker/destroy_ref.ts

@@ -6,12 +6,14 @@
  * found in the LICENSE file at https://angular.dev/license
  */
 
-import type {EnvironmentInjector} from '../di';
+import {EnvironmentInjector} from '../di';
 import {isDestroyed} from '../render3/interfaces/type_checks';
-import type {LView} from '../render3/interfaces/view';
+import {LView} from '../render3/interfaces/view';
 import {getLView} from '../render3/state';
 import {removeLViewOnDestroy, storeLViewOnDestroy} from '../render3/util/view_utils';
 
+const EXECUTE_CALLBACK_IF_ALREADY_DESTROYED = false;
+
 /**
  * `DestroyRef` lets you set callbacks to run for any cleanup or destruction behavior.
  * The scope of this destruction depends on where `DestroyRef` is injected. If `DestroyRef`
@@ -65,21 +67,9 @@ export class NodeInjectorDestroyRef extends DestroyRef {
   override onDestroy(callback: () => void): () => void {
     const lView = this._lView;
 
-    // Checking if `lView` is already destroyed before storing the `callback` enhances
-    // safety and integrity for applications.
-    // If `lView` is destroyed, we call the `callback` immediately to ensure that
-    // any necessary cleanup is handled gracefully.
-    // With this approach, we're providing better reliability in managing resources.
-    // One of the use cases is `takeUntilDestroyed`, which aims to replace `takeUntil`
-    // in existing applications. While `takeUntil` can be safely called once the view
-    // is destroyed — resulting in no errors and finalizing the subscription depending
-    // on whether a subject or replay subject is used, replacing it with
-    // `takeUntilDestroyed` introduces a breaking change, as it throws an error if
-    // the `lView` is destroyed (https://github.com/angular/angular/issues/54527).
-    if (isDestroyed(lView)) {
+    // TODO(atscott): Remove once g3 cleanup is complete
+    if (EXECUTE_CALLBACK_IF_ALREADY_DESTROYED && isDestroyed(lView)) {
       callback();
-      // We return a "noop" callback, which, when executed, does nothing because
-      // we haven't stored anything on the `lView`, and thus there's nothing to remove.
       return () => {};
     }
 

packages/core/test/acceptance/destroy_ref_spec.ts

@@ -239,6 +239,24 @@ describe('DestroyRef', () => {
     expect(onDestroyCalls).toBe(2);
   });
 
+  it('should throw when trying to register destroy callback on destroyed LView', () => {
+    @Component({
+      selector: 'test',
+      template: ``,
+    })
+    class TestCmp {
+      constructor(public destroyRef: DestroyRef) {}
+    }
+
+    const fixture = TestBed.createComponent(TestCmp);
+    const destroyRef = fixture.componentRef.instance.destroyRef;
+    fixture.componentRef.destroy();
+
+    expect(() => {
+      destroyRef.onDestroy(() => {});
+    }).toThrowError('NG0911: View has already been destroyed.');
+  });
+
   it('should allow unregistration while destroying', () => {
     const destroyedLog: string[] = [];