Skip to content

Commit

Permalink
perf(tree): leaking reference through mostRecentTreeNode (#12334)
Browse files Browse the repository at this point in the history
Along the same lines as #12269. Clears out the `mostRecentTreeNode` once the last tree node is destroyed, in order to avoid a memory leak.
  • Loading branch information
crisbeto authored and mmalerba committed Jul 24, 2018
1 parent bd76f1a commit 60b9928
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 2 deletions.
16 changes: 15 additions & 1 deletion src/cdk/tree/tree.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ import {TreeControl} from './control/tree-control';
import {FlatTreeControl} from './control/flat-tree-control';
import {NestedTreeControl} from './control/nested-tree-control';
import {CdkTreeModule} from './index';
import {CdkTree} from './tree';
import {CdkTree, CdkTreeNode} from './tree';
import {getTreeControlFunctionsMissingError} from './tree-errors';


Expand All @@ -35,6 +35,20 @@ describe('CdkTree', () => {
}).compileComponents();
}

it('should clear out the `mostRecentTreeNode` on destroy', () => {
configureCdkTreeTestingModule([SimpleCdkTreeApp]);
const fixture = TestBed.createComponent(SimpleCdkTreeApp);
fixture.detectChanges();

// Cast the assertions to a boolean to avoid Jasmine going into an
// infinite loop when stringifying the object, if the test starts failing.
expect(!!CdkTreeNode.mostRecentTreeNode).toBe(true);

fixture.destroy();

expect(!!CdkTreeNode.mostRecentTreeNode).toBe(false);
});

describe('flat tree', () => {
describe('should initialize', () => {
let fixture: ComponentFixture<SimpleCdkTreeApp>;
Expand Down
8 changes: 7 additions & 1 deletion src/cdk/tree/tree.ts
Original file line number Diff line number Diff line change
Expand Up @@ -290,7 +290,7 @@ export class CdkTree<T>
'class': 'cdk-tree-node',
},
})
export class CdkTreeNode<T> implements FocusableOption, OnDestroy {
export class CdkTreeNode<T> implements FocusableOption, OnDestroy {
/**
* The most recently created `CdkTreeNode`. We save it in static variable so we can retrieve it
* in `CdkTree` and set the data to it.
Expand Down Expand Up @@ -328,6 +328,12 @@ export class CdkTreeNode<T> implements FocusableOption, OnDestroy {
}

ngOnDestroy() {
// If this is the last tree node being destroyed,
// clear out the reference to avoid leaking memory.
if (CdkTreeNode.mostRecentTreeNode === this) {
CdkTreeNode.mostRecentTreeNode = null;
}

this._destroyed.next();
this._destroyed.complete();
}
Expand Down

0 comments on commit 60b9928

Please sign in to comment.