fix(core): error when invoking callbacks registered via ViewRef.onDes…

…troy (#37543)

Invoking a callback registered through `ViewRef.onDestroy` throws an error, because we weren't registering it correctly in the internal data structure. These changes also remove the `storeCleanupFn` function, because it was mostly identical to `storeCleanupWithContext` and was only used in one place.

Fixes #36213.

PR Close #37543

packages/core/src/render3/instructions/shared.ts

@@ -795,22 +795,6 @@ export function storeCleanupWithContext(
   }
 }
 
-/**
- * Saves the cleanup function itself in LView.cleanupInstances.
- *
- * This is necessary for functions that are wrapped with their contexts, like in renderer2
- * listeners.
- *
- * On the first template pass, the index of the cleanup function is saved in TView.
- */
-export function storeCleanupFn(tView: TView, lView: LView, cleanupFn: Function): void {
-  getLCleanup(lView).push(cleanupFn);
-
-  if (tView.firstCreatePass) {
-    getTViewCleanup(tView).push(lView[CLEANUP]!.length - 1, null);
-  }
-}
-
 /**
  * Constructs a TNode object from the arguments.
  *

packages/core/src/render3/view_ref.ts

@@ -11,7 +11,7 @@ import {ChangeDetectorRef as viewEngine_ChangeDetectorRef} from '../change_detec
 import {ViewContainerRef as viewEngine_ViewContainerRef} from '../linker/view_container_ref';
 import {EmbeddedViewRef as viewEngine_EmbeddedViewRef, InternalViewRef as viewEngine_InternalViewRef} from '../linker/view_ref';
 import {assertDefined} from '../util/assert';
-import {checkNoChangesInRootView, checkNoChangesInternal, detectChangesInRootView, detectChangesInternal, markViewDirty, storeCleanupFn} from './instructions/shared';
+import {checkNoChangesInRootView, checkNoChangesInternal, detectChangesInRootView, detectChangesInternal, markViewDirty, storeCleanupWithContext} from './instructions/shared';
 import {CONTAINER_HEADER_OFFSET} from './interfaces/container';
 import {TElementNode, TNode, TNodeType, TViewNode} from './interfaces/node';
 import {isLContainer} from './interfaces/type_checks';
@@ -88,7 +88,7 @@ export class ViewRef<T> implements viewEngine_EmbeddedViewRef<T>, viewEngine_Int
   }
 
   onDestroy(callback: Function) {
-    storeCleanupFn(this._lView[TVIEW], this._lView, callback);
+    storeCleanupWithContext(this._lView[TVIEW], this._lView, null, callback);
   }
 
   /**

packages/core/test/acceptance/view_ref_spec.ts

@@ -6,7 +6,7 @@
  * found in the LICENSE file at https://angular.io/license
  */
 
-import {ApplicationRef, Component, ComponentFactoryResolver, ComponentRef, ElementRef, Injector, NgModule} from '@angular/core';
+import {ApplicationRef, ChangeDetectorRef, Component, ComponentFactoryResolver, ComponentRef, ElementRef, Injector, NgModule} from '@angular/core';
 import {InternalViewRef} from '@angular/core/src/linker/view_ref';
 import {TestBed} from '@angular/core/testing';
 
@@ -54,4 +54,22 @@ describe('ViewRef', () => {
     fixture.detectChanges();
     expect(document.body.querySelector('dynamic-cpt')).toBeFalsy();
   });
+
+  it('should invoke the onDestroy callback of a view ref', () => {
+    let called = false;
+
+    @Component({template: ''})
+    class App {
+      constructor(changeDetectorRef: ChangeDetectorRef) {
+        (changeDetectorRef as InternalViewRef).onDestroy(() => called = true);
+      }
+    }
+
+    TestBed.configureTestingModule({declarations: [App]});
+    const fixture = TestBed.createComponent(App);
+    fixture.detectChanges();
+    fixture.destroy();
+
+    expect(called).toBe(true);
+  });
 });