Skip to content

Commit 0e21443

Browse files
crisbetommalerba
authored andcommitted
fix(overlay): clear element reference on destroy (#10486)
Clears out the references to the overlay element once the overlay ref is destroyed, avoiding the detached DOM tree potentially being retained in memory. This also revealed a few tests where we were referring to leftover overlay refs from previous tests.
1 parent 153dfb8 commit 0e21443

File tree

5 files changed

+27
-7
lines changed

5 files changed

+27
-7
lines changed

src/cdk/overlay/overlay-ref.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -186,6 +186,8 @@ export class OverlayRef implements PortalOutlet {
186186
this._host = null!;
187187
}
188188

189+
this._pane = null!;
190+
189191
if (isAttached) {
190192
this._detachments.next();
191193
}

src/cdk/overlay/overlay.spec.ts

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -259,7 +259,7 @@ describe('Overlay', () => {
259259
});
260260

261261
it('should default to the ltr direction', () => {
262-
const overlayRef = overlay.create({hasBackdrop: true});
262+
const overlayRef = overlay.create();
263263
expect(overlayRef.getConfig().direction).toBe('ltr');
264264
});
265265

@@ -268,6 +268,22 @@ describe('Overlay', () => {
268268
expect(overlayRef.getConfig().direction).toBe('ltr');
269269
});
270270

271+
it('should clear out all DOM element references on dispose', fakeAsync(() => {
272+
const overlayRef = overlay.create({hasBackdrop: true});
273+
overlayRef.attach(componentPortal);
274+
275+
expect(overlayRef.hostElement).toBeTruthy('Expected overlay host to be defined.');
276+
expect(overlayRef.overlayElement).toBeTruthy('Expected overlay element to be defined.');
277+
expect(overlayRef.backdropElement).toBeTruthy('Expected backdrop element to be defined.');
278+
279+
overlayRef.dispose();
280+
tick(500);
281+
282+
expect(overlayRef.hostElement).toBeFalsy('Expected overlay host not to be referenced.');
283+
expect(overlayRef.overlayElement).toBeFalsy('Expected overlay element not to be referenced.');
284+
expect(overlayRef.backdropElement).toBeFalsy('Expected backdrop element not to be referenced.');
285+
}));
286+
271287
describe('positioning', () => {
272288
let config: OverlayConfig;
273289

src/cdk/overlay/position/connected-position-strategy.spec.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -280,7 +280,6 @@ describe('ConnectedPositionStrategy', () => {
280280

281281
it('should position a panel properly when rtl', () => {
282282
// must make the overlay longer than the origin to properly test attachment
283-
overlayRef.overlayElement.style.width = `500px`;
284283
originRect = originElement.getBoundingClientRect();
285284
positionStrategy = overlay.position().connectedTo(
286285
fakeElementRef,
@@ -289,6 +288,7 @@ describe('ConnectedPositionStrategy', () => {
289288
.withDirection('rtl');
290289

291290
attachOverlay(positionStrategy);
291+
overlayRef.overlayElement.style.width = `500px`;
292292

293293
let overlayRect = overlayRef.overlayElement.getBoundingClientRect();
294294
expect(Math.floor(overlayRect.top)).toBe(Math.floor(originRect.bottom));

src/cdk/overlay/position/flexible-connected-position-strategy.spec.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -344,9 +344,6 @@ describe('FlexibleConnectedPositionStrategy', () => {
344344
});
345345

346346
it('should position a panel properly when rtl', () => {
347-
// must make the overlay longer than the origin to properly test attachment
348-
overlayRef.overlayElement.style.width = `500px`;
349-
350347
const originRect = originElement.getBoundingClientRect();
351348

352349
positionStrategy.withPositions([{
@@ -361,6 +358,9 @@ describe('FlexibleConnectedPositionStrategy', () => {
361358
direction: 'rtl'
362359
});
363360

361+
// must make the overlay longer than the origin to properly test attachment
362+
overlayRef.overlayElement.style.width = `500px`;
363+
364364
const overlayRect = overlayRef.overlayElement.getBoundingClientRect();
365365
expect(Math.floor(overlayRect.top)).toBe(Math.floor(originRect.bottom));
366366
expect(Math.floor(overlayRect.right)).toBe(Math.floor(originRect.right));

src/cdk/overlay/position/global-position-strategy.spec.ts

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -166,11 +166,13 @@ describe('GlobalPositonStrategy', () => {
166166
positionStrategy: overlay.position().global()
167167
});
168168

169-
expect(document.body.contains(overlayRef.overlayElement.parentNode!)).toBe(true);
169+
const parent = overlayRef.overlayElement.parentNode!;
170+
171+
expect(document.body.contains(parent)).toBe(true);
170172

171173
overlayRef.dispose();
172174

173-
expect(document.body.contains(overlayRef.overlayElement.parentNode!)).toBe(false);
175+
expect(document.body.contains(parent)).toBe(false);
174176
});
175177

176178
it('should set the element width', () => {

0 commit comments

Comments
 (0)