Skip to content

Commit 46b160e

Browse files
crisbetoAndrewKushnir
authored andcommittedJul 29, 2019
fix(core): DebugElement.listeners not cleared on destroy (#31820)
Currently the `DebugElement.listeners` array are retained after the node is destroyed. This means that they'll continue to fire through `triggerEventHandler` and can cause memory leaks. This has already been fixed in Ivy, but these changes fix it in ViewEngine for consistency. PR Close #31820
·
8.2.148.2.0
1 parent dcbc28f commit 46b160e

File tree

2 files changed

+41
-2
lines changed

2 files changed

+41
-2
lines changed
 

‎packages/core/src/view/services.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -688,7 +688,11 @@ export class DebugRenderer2 implements Renderer2 {
688688
constructor(private delegate: Renderer2) { this.data = this.delegate.data; }
689689

690690
destroyNode(node: any) {
691-
removeDebugNodeFromIndex(getDebugNode(node) !);
691+
const debugNode = getDebugNode(node) !;
692+
removeDebugNodeFromIndex(debugNode);
693+
if (debugNode instanceof DebugNode__PRE_R3__) {
694+
debugNode.listeners.length = 0;
695+
}
692696
if (this.delegate.destroyNode) {
693697
this.delegate.destroyNode(node);
694698
}

‎packages/core/test/debug/debug_node_spec.ts

Lines changed: 36 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88

99

1010
import {CommonModule, NgIfContext} from '@angular/common';
11-
import {Component, DebugNode, Directive, ElementRef, EmbeddedViewRef, EventEmitter, HostBinding, Injectable, Input, NO_ERRORS_SCHEMA, Renderer2, TemplateRef, ViewChild, ViewContainerRef} from '@angular/core';
11+
import {Component, DebugNode, Directive, ElementRef, EmbeddedViewRef, EventEmitter, HostBinding, Injectable, Input, NO_ERRORS_SCHEMA, Output, Renderer2, TemplateRef, ViewChild, ViewContainerRef} from '@angular/core';
1212
import {ComponentFixture, TestBed, async} from '@angular/core/testing';
1313
import {By} from '@angular/platform-browser/src/dom/debug/by';
1414
import {getDOM} from '@angular/platform-browser/src/dom/dom_adapter';
@@ -927,5 +927,40 @@ class TestCmptWithPropBindings {
927927
expect(div.attributes.foo).toBe('bar');
928928
});
929929

930+
it('should clear event listeners when node is destroyed', () => {
931+
let calls = 0;
932+
@Component({
933+
selector: 'cancel-button',
934+
template: '',
935+
})
936+
class CancelButton {
937+
@Output() cancel = new EventEmitter<void>();
938+
}
939+
940+
@Component({
941+
template: '<cancel-button *ngIf="visible" (cancel)="cancel()"></cancel-button>',
942+
})
943+
class App {
944+
visible = true;
945+
cancel() { calls++; }
946+
}
947+
948+
TestBed.configureTestingModule({declarations: [App, CancelButton]});
949+
const fixture = TestBed.createComponent(App);
950+
fixture.detectChanges();
951+
952+
const button = fixture.debugElement.query(By.directive(CancelButton));
953+
button.triggerEventHandler('cancel', {});
954+
955+
expect(calls).toBe(1, 'Expected calls to be 1 after one event.');
956+
957+
fixture.componentInstance.visible = false;
958+
fixture.detectChanges();
959+
960+
button.triggerEventHandler('cancel', {});
961+
962+
expect(calls).toBe(1, 'Expected calls to stay 1 after destroying the node.');
963+
});
964+
930965
});
931966
}

0 commit comments

Comments
 (0)
Please sign in to comment.