fix(core): Call onDestroy in production mode as well

PR angular#39876 introduced an error where the `onDestroy` of `ComponentRef` would only get called if `ngDevMode` was set to true. This was because in dev mode we would freeze `TCleanup` to verify that no more static cleanup would get added to `TCleanup` array. This ensured that `TCleanup` was always present in dev mode. In production the `TCleanup` would get created only when needed. The resulting cleanup code was incorrectly indented and would only run if `TCleanup` was present causing this issue. Fix angular#40105
mhevery · Dec 14, 2020 · 2f66761 · 2f66761
1 parent 2a74431
commit 2f66761
Show file tree

Hide file tree

Showing 2 changed files with 68 additions and 55 deletions.
diff --git a/packages/core/src/render3/node_manipulation.ts b/packages/core/src/render3/node_manipulation.ts
@@ -479,12 +479,12 @@ function processCleanups(tView: TView, lView: LView): void {
         tCleanup[i].call(context);
       }
     }
-    if (lCleanup !== null) {
-      for (let i = lastLCleanupIndex + 1; i < lCleanup.length; i++) {
-        const instanceCleanupFn = lCleanup[i];
-        ngDevMode && assertFunction(instanceCleanupFn, 'Expecting instance cleanup function.');
-        instanceCleanupFn();
-      }
+  }
+  if (lCleanup !== null) {
+    for (let i = lastLCleanupIndex + 1; i < lCleanup.length; i++) {
+      const instanceCleanupFn = lCleanup[i];
+      ngDevMode && assertFunction(instanceCleanupFn, 'Expecting instance cleanup function.');
+      instanceCleanupFn();
     }
     lView[CLEANUP] = null;
   }

diff --git a/packages/core/test/acceptance/component_spec.ts b/packages/core/test/acceptance/component_spec.ts
@@ -303,57 +303,70 @@ describe('component', () => {
        expect(wrapperEls.length).toBe(2);  // other elements are preserved
      });
 
-  it('should invoke `onDestroy` callbacks of dynamically created component', () => {
-    let wasOnDestroyCalled = false;
-    @Component({
-      selector: '[comp]',
-      template: 'comp content',
-    })
-    class DynamicComponent {
-    }
-
-    @NgModule({
-      declarations: [DynamicComponent],
-      entryComponents: [DynamicComponent],  // needed only for ViewEngine
-    })
-    class TestModule {
-    }
-
-    @Component({
-      selector: 'button',
-      template: '<div id="app-root" #anchor></div>',
-    })
-    class App {
-      @ViewChild('anchor', {read: ViewContainerRef}) anchor!: ViewContainerRef;
-
-      constructor(private cfr: ComponentFactoryResolver, private injector: Injector) {}
-
-      create() {
-        const factory = this.cfr.resolveComponentFactory(DynamicComponent);
-        const componentRef = factory.create(this.injector);
-        componentRef.onDestroy(() => {
-          wasOnDestroyCalled = true;
-        });
-        this.anchor.insert(componentRef.hostView);
-      }
-
-      clear() {
-        this.anchor.clear();
-      }
-    }
-
-    TestBed.configureTestingModule({imports: [TestModule], declarations: [App]});
-    const fixture = TestBed.createComponent(App);
-    fixture.detectChanges();
-
-    // Add ComponentRef to ViewContainerRef instance.
-    fixture.componentInstance.create();
-    // Clear ViewContainerRef to invoke `onDestroy` callbacks on ComponentRef.
-    fixture.componentInstance.clear();
-
-    expect(wasOnDestroyCalled).toBeTrue();
+  describe('with ngDevMode', () => {
+    const _global: {ngDevMode: any} = global as any;
+    let saveNgDevMode!: typeof ngDevMode;
+    beforeEach(() => saveNgDevMode = ngDevMode);
+    afterEach(() => _global.ngDevMode = saveNgDevMode);
+    ['ngDevMode off', 'ngDevMode on'].forEach((mode) => {
+      it('should invoke `onDestroy` callbacks of dynamically created component with ' + mode,
+         () => {
+           if (mode === 'ngDevMode off') {
+             _global.ngDevMode = false;
+           }
+           let wasOnDestroyCalled = false;
+           @Component({
+             selector: '[comp]',
+             template: 'comp content',
+           })
+           class DynamicComponent {
+           }
+
+           @NgModule({
+             declarations: [DynamicComponent],
+             entryComponents: [DynamicComponent],  // needed only for ViewEngine
+           })
+           class TestModule {
+           }
+
+           @Component({
+             selector: 'button',
+             template: '<div id="app-root" #anchor></div>',
+           })
+           class App {
+             @ViewChild('anchor', {read: ViewContainerRef}) anchor!: ViewContainerRef;
+
+             constructor(private cfr: ComponentFactoryResolver, private injector: Injector) {}
+
+             create() {
+               const factory = this.cfr.resolveComponentFactory(DynamicComponent);
+               const componentRef = factory.create(this.injector);
+               componentRef.onDestroy(() => {
+                 wasOnDestroyCalled = true;
+               });
+               this.anchor.insert(componentRef.hostView);
+             }
+
+             clear() {
+               this.anchor.clear();
+             }
+           }
+
+           TestBed.configureTestingModule({imports: [TestModule], declarations: [App]});
+           const fixture = TestBed.createComponent(App);
+           fixture.detectChanges();
+
+           // Add ComponentRef to ViewContainerRef instance.
+           fixture.componentInstance.create();
+           // Clear ViewContainerRef to invoke `onDestroy` callbacks on ComponentRef.
+           fixture.componentInstance.clear();
+
+           expect(wasOnDestroyCalled).toBeTrue();
+         });
+    });
   });
 
+
   describe('invalid host element', () => {
     it('should throw when <ng-container> is used as a host element for a Component', () => {
       @Component({