Skip to content

Commit d387183

Browse files
Fixed a memory leak when placing a DxSlider or DxButton inside Angular template @if (T1324584) (DevExpress#33040)
1 parent f549caf commit d387183

File tree

4 files changed

+159
-0
lines changed

4 files changed

+159
-0
lines changed
Lines changed: 104 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,104 @@
1+
import { Component } from '@angular/core';
2+
import { TestBed } from '@angular/core/testing';
3+
import { DxButtonModule, DxSliderModule } from 'devextreme-angular';
4+
5+
@Component({
6+
standalone: false,
7+
selector: 'test-container-component',
8+
template: '',
9+
})
10+
class TestContainerComponent {
11+
isVisible = true;
12+
}
13+
14+
async function forceGC(times = 3): Promise<void> {
15+
for (let i = 0; i < times; i++) {
16+
Array.from({ length: 10_000 }, () => ({ data: new Array(100) }));
17+
globalThis.gc?.();
18+
}
19+
20+
await new Promise((resolve) => {
21+
setTimeout(resolve, 0);
22+
});
23+
}
24+
25+
describe('Memory leak tests', () => {
26+
const originalTimeout = jasmine.DEFAULT_TIMEOUT_INTERVAL;
27+
28+
beforeAll(() => {
29+
jasmine.DEFAULT_TIMEOUT_INTERVAL = 30000;
30+
31+
TestBed.configureTestingModule({
32+
declarations: [TestContainerComponent],
33+
imports: [DxButtonModule, DxSliderModule],
34+
});
35+
});
36+
37+
afterAll(() => {
38+
jasmine.DEFAULT_TIMEOUT_INTERVAL = originalTimeout;
39+
});
40+
41+
it('should not memory leak after change @if block with DxButton (T1324584)', async () => {
42+
TestBed.overrideComponent(TestContainerComponent, {
43+
set: {
44+
template: '@if (isVisible) {<dx-button text="BUTTON"></dx-button>}',
45+
},
46+
});
47+
48+
const fixture = TestBed.createComponent(TestContainerComponent);
49+
const component: TestContainerComponent = fixture.componentInstance;
50+
51+
fixture.detectChanges();
52+
53+
for (let i = 0; i < 100; i++) {
54+
component.isVisible = !component.isVisible;
55+
fixture.detectChanges();
56+
}
57+
58+
const memoryBefore = await (performance as any).measureUserAgentSpecificMemory();
59+
60+
for (let i = 0; i < 100; i++) {
61+
component.isVisible = !component.isVisible;
62+
fixture.detectChanges();
63+
}
64+
65+
await forceGC();
66+
67+
const memoryAfter = await (performance as any).measureUserAgentSpecificMemory();
68+
const memoryDiff = Math.round((memoryAfter.bytes - memoryBefore.bytes) / 1024);
69+
70+
expect(memoryDiff).toBeLessThan(150);
71+
});
72+
73+
it('should not memory leak after change @if block with DxSlider (T1324584)', async () => {
74+
TestBed.overrideComponent(TestContainerComponent, {
75+
set: {
76+
template: '@if (isVisible) {<dx-slider></dx-slider>}',
77+
},
78+
});
79+
80+
const fixture = TestBed.createComponent(TestContainerComponent);
81+
const component: TestContainerComponent = fixture.componentInstance;
82+
83+
fixture.detectChanges();
84+
85+
for (let i = 0; i < 100; i++) {
86+
component.isVisible = !component.isVisible;
87+
fixture.detectChanges();
88+
}
89+
90+
const memoryBefore = await (performance as any).measureUserAgentSpecificMemory();
91+
92+
for (let i = 0; i < 100; i++) {
93+
component.isVisible = !component.isVisible;
94+
fixture.detectChanges();
95+
}
96+
97+
await forceGC();
98+
99+
const memoryAfter = await (performance as any).measureUserAgentSpecificMemory();
100+
const memoryDiff = Math.round((memoryAfter.bytes - memoryBefore.bytes) / 1024);
101+
102+
expect(memoryDiff).toBeLessThan(200);
103+
});
104+
});

packages/devextreme/js/__internal/core/m_inferno_renderer.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
/* eslint-disable spellcheck/spell-checker */
2+
import { keyboard } from '@js/common/core/events/short';
23
import domAdapter from '@js/core/dom_adapter';
34
import { cleanDataRecursive } from '@js/core/element_data';
45
import injector from '@js/core/utils/dependency_injector';
@@ -7,6 +8,8 @@ import { render } from 'inferno';
78
import { createElement } from 'inferno-create-element';
89

910
const remove = (element) => {
11+
keyboard.disposeProcessorsForSubtree(element);
12+
1013
const { parentNode } = element;
1114

1215
if (parentNode) {

packages/devextreme/js/__internal/events/m_short.ts

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -131,6 +131,57 @@ export const keyboard = {
131131
}
132132
},
133133

134+
disposeProcessorsForSubtree(root: Element): void {
135+
if (!root?.nodeType) {
136+
return;
137+
}
138+
139+
const toElements = (value: unknown): Element[] => {
140+
if (!value) {
141+
return [];
142+
}
143+
144+
if (value instanceof Element) {
145+
return [value];
146+
}
147+
148+
if (Array.isArray(value)) {
149+
return value.filter((item): item is Element => item instanceof Element);
150+
}
151+
152+
const v = value as { toArray?: () => unknown[]; 0?: unknown };
153+
154+
if (typeof v.toArray === 'function') {
155+
const arr = v.toArray();
156+
157+
if (Array.isArray(arr)) {
158+
return arr.filter((item): item is Element => item instanceof Element);
159+
}
160+
}
161+
162+
const first = v[0];
163+
164+
return first instanceof Element ? [first] : [];
165+
};
166+
167+
const touchesRoot = (value: unknown): boolean => {
168+
const elements = toElements(value);
169+
return elements.some((el) => el === root || root.contains(el));
170+
};
171+
172+
Object.keys(keyboardProcessors).forEach((id) => {
173+
const keyboardProcessor = keyboardProcessors[id];
174+
175+
if (!keyboardProcessor) {
176+
return;
177+
}
178+
179+
if (touchesRoot(keyboardProcessor._element) || touchesRoot(keyboardProcessor._focusTarget)) {
180+
keyboard.off(id);
181+
}
182+
});
183+
},
184+
134185
// NOTE: For tests
135186
_getProcessor: (listenerId) => keyboardProcessors[listenerId],
136187
};

packages/devextreme/js/__internal/ui/slider/m_slider_handle.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,7 @@ class SliderHandle extends Widget<SliderHandlerProperties> {
7474

7575
_clean(): void {
7676
super._clean();
77+
this._sliderTooltip?.dispose();
7778
this._sliderTooltip = null;
7879
}
7980

0 commit comments

Comments
 (0)