Skip to content

Commit 334a111

Browse files
authored
Fix memory leak due to focus listeners not being deleted (#143)
* Fix memory leak due to focus listeners not being deleted * Remove event listener on container on stop
1 parent 4cd3fa3 commit 334a111

File tree

1 file changed

+14
-26
lines changed

1 file changed

+14
-26
lines changed

src/Virtual.ts

Lines changed: 14 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -205,6 +205,7 @@ export class Virtual {
205205
#spokenPhraseLog: string[] = [];
206206
#treeCache: AccessibilityNode[] | null = null;
207207
#disconnectDOMObserver: (() => void) | null = null;
208+
#boundHandleFocusChange: ((event: Event) => Promise<void>) | null = null;
208209

209210
#checkContainer() {
210211
if (!this.#container) {
@@ -257,8 +258,6 @@ export class Virtual {
257258

258259
this.#treeCache =
259260
this.#container && tree ? flattenTree(this.#container, tree, null) : [];
260-
261-
this.#attachFocusListeners();
262261
}
263262

264263
return this.#treeCache;
@@ -290,28 +289,9 @@ export class Virtual {
290289
}
291290

292291
#invalidateTreeCache() {
293-
this.#detachFocusListeners();
294292
this.#treeCache = null;
295293
}
296294

297-
#attachFocusListeners() {
298-
this.#getAccessibilityTree().forEach((treeNode) => {
299-
treeNode.node.addEventListener(
300-
"focus",
301-
this.#handleFocusChange.bind(this)
302-
);
303-
});
304-
}
305-
306-
#detachFocusListeners() {
307-
this.#getAccessibilityTree().forEach((treeNode) => {
308-
treeNode.node.removeEventListener(
309-
"focus",
310-
this.#handleFocusChange.bind(this)
311-
);
312-
});
313-
}
314-
315295
async #handleFocusChange({ target }: Event) {
316296
await tick();
317297

@@ -322,10 +302,13 @@ export class Virtual {
322302
return;
323303
}
324304

325-
// We've covered the tree having no length so there must be at least one
326-
// index or we default back to the beginning of the tree.
327-
const newActiveNode =
328-
tree.find(({ node }) => node === target) ?? tree.at(0)!;
305+
// We've covered the tree having no length so there should be at least one
306+
// matching node, but if not we will not update the state
307+
const newActiveNode = tree.find(({ node }) => node === target);
308+
309+
if (!newActiveNode) {
310+
return;
311+
}
329312

330313
this.#updateState(newActiveNode, true);
331314
}
@@ -619,6 +602,10 @@ export class Virtual {
619602
return;
620603
}
621604

605+
this.#boundHandleFocusChange = this.#handleFocusChange.bind(this);
606+
607+
this.#container.addEventListener("focusin", this.#boundHandleFocusChange);
608+
622609
this.#updateState(tree[0]);
623610

624611
return;
@@ -647,6 +634,7 @@ export class Virtual {
647634
*/
648635
async stop() {
649636
this.#disconnectDOMObserver?.();
637+
this.#container?.removeEventListener("focusin", this.#boundHandleFocusChange);
650638
this.#invalidateTreeCache();
651639

652640
if (this.#cursor) {
@@ -658,7 +646,7 @@ export class Virtual {
658646
this.#container = null;
659647
this.#itemTextLog = [];
660648
this.#spokenPhraseLog = [];
661-
649+
this.#boundHandleFocusChange = null;
662650
return;
663651
}
664652

0 commit comments

Comments
 (0)