Skip to content

Commit 13b441d

Browse files
committed
fix(tab-bar): prevent keyboard controller memory leak on rapid mount/unmount
1 parent 07b46d7 commit 13b441d

File tree

3 files changed

+204
-1
lines changed

3 files changed

+204
-1
lines changed

core/src/components/tab-bar/tab-bar.tsx

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ import type { TabBarChangedEventDetail } from './tab-bar-interface';
2323
export class TabBar implements ComponentInterface {
2424
private keyboardCtrl: KeyboardController | null = null;
2525
private didLoad = false;
26+
private isComponentConnected = false;
2627

2728
@Element() el!: HTMLElement;
2829

@@ -88,7 +89,9 @@ export class TabBar implements ComponentInterface {
8889
}
8990

9091
async connectedCallback() {
91-
this.keyboardCtrl = await createKeyboardController(async (keyboardOpen, waitForResize) => {
92+
this.isComponentConnected = true;
93+
94+
const keyboardCtrl = await createKeyboardController(async (keyboardOpen, waitForResize) => {
9295
/**
9396
* If the keyboard is hiding, then we need to wait
9497
* for the webview to resize. Otherwise, the tab bar
@@ -100,11 +103,24 @@ export class TabBar implements ComponentInterface {
100103

101104
this.keyboardVisible = keyboardOpen; // trigger re-render by updating state
102105
});
106+
107+
/**
108+
* Destroy the keyboard controller if the component was
109+
* disconnected during async initialization to prevent memory leaks.
110+
*/
111+
if (this.isComponentConnected) {
112+
this.keyboardCtrl = keyboardCtrl;
113+
} else {
114+
keyboardCtrl.destroy();
115+
}
103116
}
104117

105118
disconnectedCallback() {
119+
this.isComponentConnected = false;
120+
106121
if (this.keyboardCtrl) {
107122
this.keyboardCtrl.destroy();
123+
this.keyboardCtrl = null;
108124
}
109125
}
110126

Lines changed: 121 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,121 @@
1+
import { expect } from '@playwright/test';
2+
import { configs, test } from '@utils/test/playwright';
3+
4+
/**
5+
* Tests for keyboard controller memory leak fix when tab-bar
6+
* is rapidly mounted/unmounted.
7+
*/
8+
configs({ modes: ['ios'], directions: ['ltr'] }).forEach(({ title, config }) => {
9+
test.describe(title('tab-bar: lifecycle'), () => {
10+
test('should not error when rapidly mounting and unmounting', async ({ page }) => {
11+
const errors: string[] = [];
12+
page.on('console', (msg) => {
13+
if (msg.type() === 'error') {
14+
errors.push(msg.text());
15+
}
16+
});
17+
18+
await page.setContent(
19+
`
20+
<div id="container"></div>
21+
<script>
22+
const container = document.getElementById('container');
23+
24+
for (let i = 0; i < 10; i++) {
25+
const tabBar = document.createElement('ion-tab-bar');
26+
tabBar.innerHTML = \`
27+
<ion-tab-button tab="tab1">
28+
<ion-label>Tab 1</ion-label>
29+
</ion-tab-button>
30+
\`;
31+
container.appendChild(tabBar);
32+
container.removeChild(tabBar);
33+
}
34+
</script>
35+
`,
36+
config
37+
);
38+
39+
await page.waitForTimeout(500);
40+
41+
expect(errors.length).toBe(0);
42+
});
43+
44+
test('should cleanup keyboard controller when removed from DOM', async ({ page }, testInfo) => {
45+
testInfo.annotations.push({
46+
type: 'issue',
47+
description: 'IONIC-82',
48+
});
49+
50+
const errors: string[] = [];
51+
page.on('console', (msg) => {
52+
if (msg.type() === 'error') {
53+
errors.push(msg.text());
54+
}
55+
});
56+
57+
await page.setContent(
58+
`
59+
<div id="container">
60+
<ion-tab-bar id="test-tab-bar">
61+
<ion-tab-button tab="tab1">
62+
<ion-label>Tab 1</ion-label>
63+
</ion-tab-button>
64+
</ion-tab-bar>
65+
</div>
66+
`,
67+
config
68+
);
69+
70+
const tabBar = page.locator('#test-tab-bar');
71+
await expect(tabBar).toBeVisible();
72+
73+
await page.evaluate(() => {
74+
document.getElementById('test-tab-bar')?.remove();
75+
});
76+
77+
await page.waitForTimeout(100);
78+
79+
await expect(tabBar).not.toBeAttached();
80+
expect(errors.length).toBe(0);
81+
});
82+
83+
test('should handle re-mounting after unmount', async ({ page }) => {
84+
await page.setContent(
85+
`
86+
<div id="container"></div>
87+
<script>
88+
const container = document.getElementById('container');
89+
90+
async function testRemount() {
91+
const tabBar = document.createElement('ion-tab-bar');
92+
tabBar.id = 'remount-tab-bar';
93+
tabBar.innerHTML = \`
94+
<ion-tab-button tab="tab1">
95+
<ion-label>Tab 1</ion-label>
96+
</ion-tab-button>
97+
\`;
98+
container.appendChild(tabBar);
99+
100+
await new Promise(r => setTimeout(r, 100));
101+
102+
container.removeChild(tabBar);
103+
104+
await new Promise(r => setTimeout(r, 50));
105+
106+
container.appendChild(tabBar);
107+
}
108+
109+
testRemount();
110+
</script>
111+
`,
112+
config
113+
);
114+
115+
await page.waitForTimeout(500);
116+
117+
const tabBar = page.locator('#remount-tab-bar');
118+
await expect(tabBar).toBeVisible();
119+
});
120+
});
121+
});
Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,66 @@
1+
import { newSpecPage } from '@stencil/core/testing';
2+
3+
import { TabBar } from '../tab-bar';
4+
import { TabButton } from '../../tab-button/tab-button';
5+
6+
describe('tab-bar', () => {
7+
describe('basic rendering', () => {
8+
it('should render with tab buttons', async () => {
9+
const page = await newSpecPage({
10+
components: [TabBar, TabButton],
11+
html: `
12+
<ion-tab-bar>
13+
<ion-tab-button tab="tab1">
14+
<ion-label>Tab 1</ion-label>
15+
</ion-tab-button>
16+
</ion-tab-bar>
17+
`,
18+
});
19+
20+
expect(page.root).toBeTruthy();
21+
expect(page.root?.tagName).toBe('ION-TAB-BAR');
22+
});
23+
24+
it('should have correct role attribute', async () => {
25+
const page = await newSpecPage({
26+
components: [TabBar],
27+
html: `<ion-tab-bar></ion-tab-bar>`,
28+
});
29+
30+
expect(page.root?.getAttribute('role')).toBe('tablist');
31+
});
32+
});
33+
34+
describe('lifecycle', () => {
35+
it('should handle removal without errors', async () => {
36+
const page = await newSpecPage({
37+
components: [TabBar],
38+
html: `<ion-tab-bar></ion-tab-bar>`,
39+
});
40+
41+
page.root?.remove();
42+
await page.waitForChanges();
43+
44+
expect(page.body.querySelector('ion-tab-bar')).toBeNull();
45+
});
46+
47+
it('should handle rapid mount/unmount without errors', async () => {
48+
const page = await newSpecPage({
49+
components: [TabBar],
50+
html: `<div id="container"></div>`,
51+
});
52+
53+
const container = page.body.querySelector('#container')!;
54+
55+
for (let i = 0; i < 3; i++) {
56+
const tabBar = page.doc.createElement('ion-tab-bar');
57+
container.appendChild(tabBar);
58+
await page.waitForChanges();
59+
tabBar.remove();
60+
await page.waitForChanges();
61+
}
62+
63+
expect(container.children.length).toBe(0);
64+
});
65+
});
66+
});

0 commit comments

Comments
 (0)