Skip to content

Commit

Permalink
fix(core): remove nested views from registry when root is destroyed
Browse files Browse the repository at this point in the history
This is a follow-up to angular#41358 where my assumption was that `destroyLView` would be called for all views down the tree, but it turns out that it only gets called for the root and we actually call `cleanUpView` for the descendants. The result is that some views might not be removed from the registry after they're destroyed.

These changes move the `unregisterLView` call into `cleanUpView` instead.
  • Loading branch information
crisbeto committed Apr 30, 2021
1 parent ea89617 commit 9221329
Show file tree
Hide file tree
Showing 2 changed files with 45 additions and 2 deletions.
4 changes: 3 additions & 1 deletion packages/core/src/render3/node_manipulation.ts
Expand Up @@ -396,7 +396,6 @@ export function destroyLView(tView: TView, lView: LView) {
}

destroyViewTree(lView);
unregisterLView(lView);
}
}

Expand Down Expand Up @@ -443,6 +442,9 @@ function cleanUpView(tView: TView, lView: LView): void {
lQueries.detachView(tView);
}
}

// Deregister the view once everything else has been cleaned up.
unregisterLView(lView);
}
}

Expand Down
43 changes: 42 additions & 1 deletion packages/core/test/acceptance/integration_spec.ts
Expand Up @@ -11,7 +11,11 @@ import {MockAnimationDriver, MockAnimationPlayer} from '@angular/animations/brow
import {CommonModule} from '@angular/common';
import {Component, ContentChild, Directive, ElementRef, EventEmitter, HostBinding, HostListener, Input, NgModule, OnInit, Output, Pipe, QueryList, TemplateRef, ViewChild, ViewChildren, ViewContainerRef} from '@angular/core';
import {Inject} from '@angular/core/src/di';
import {TVIEW} from '@angular/core/src/render3/interfaces/view';
import {readPatchedLView} from '@angular/core/src/render3/context_discovery';
import {LContainer} from '@angular/core/src/render3/interfaces/container';
import {getLViewById} from '@angular/core/src/render3/interfaces/lview_tracking';
import {isLView} from '@angular/core/src/render3/interfaces/type_checks';
import {ID, LView, PARENT, TVIEW} from '@angular/core/src/render3/interfaces/view';
import {getLView} from '@angular/core/src/render3/state';
import {ngDevModeResetPerfCounters} from '@angular/core/src/util/ng_dev_mode';
import {fakeAsync, flushMicrotasks, TestBed} from '@angular/core/testing';
Expand Down Expand Up @@ -2011,6 +2015,43 @@ describe('acceptance integration tests', () => {
expect(fixture.nativeElement.innerHTML).toContain('<text>Hello</text>');
});

onlyInIvy('The test is checking Ivy-specific logic')
.it('should remove child LView from the registry when the root view is destroyed', () => {
@Component({template: '<child></child>'})
class App {
}

@Component({selector: 'child', template: '<grand-child></grand-child>'})
class Child {
}

@Component({selector: 'grand-child', template: ''})
class GrandChild {
}

TestBed.configureTestingModule({declarations: [App, Child, GrandChild]});
const fixture = TestBed.createComponent(App);
const grandChild = fixture.debugElement.query(By.directive(GrandChild)).componentInstance;
fixture.detectChanges();
const leafLView = readPatchedLView(grandChild)!;
const lViewIds: number[] = [];
let current: LView|LContainer|null = leafLView;

while (current) {
isLView(current) && lViewIds.push(current[ID]);
current = current[PARENT];
}

// We expect 3 views: `GrandChild`, `Child` and `App`.
expect(lViewIds).toEqual([leafLView[ID], leafLView[ID] - 1, leafLView[ID] - 2]);
expect(lViewIds.every(id => getLViewById(id) !== null)).toBe(true);

fixture.destroy();

// Expect all 3 views to be removed from the registry once the root is destroyed.
expect(lViewIds.map(getLViewById)).toEqual([null, null, null]);
});

describe('tView.firstUpdatePass', () => {
function isFirstUpdatePass() {
const lView = getLView();
Expand Down

0 comments on commit 9221329

Please sign in to comment.