microsoft / microsoft/polyfills

focusgroup: a group is never disconnected when its owner is removed along with an ancestor

Open
#72 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
JavaScript
Stars
8
Forks
4
Avg merge
2d 1h
Merged PRs (30d)
2

Description

Package: @microsoft/focusgroup-polyfill 1.6.0 (default and shadowless bundles)

Bug

When a focusgroup owner leaves the DOM because one of its ancestors was removed, the polyfill keeps it inside it's elementPolyfillMap. Its FocusGroup is never disconnected, and its item collection's MutationObserver stays alive. This keeps the detached subtree from being garbage-collected.

Removing the owner element itself works as expected.

As far as I can tell, this is how react usually handles removing components, so every change to unmount leaks a group.

Repro

Run in a browser without native focusgroup (Firefox, or Chrome without the flag):

<!doctype html>
<meta charset="utf-8">
<div id="a"><div focusgroup="toolbar"><button>A1</button><button>A2</button></div></div>
<div id="b"><div focusgroup="toolbar"><button>B1</button><button>B2</button></div></div>
<pre id="out"></pre>

<script type="module">
import { polyfill } from "https://cdn.jsdelivr.net/npm/@microsoft/focusgroup-polyfill@1.6.0/+esm";

const state = globalThis.__FOCUSGROUP_POLYFILL__;
const log = (line) => { document.getElementById("out").textContent += `${line}\n`; };
const nextFrame = () => new Promise((resolve) => requestAnimationFrame(resolve));
const settle = () => new Promise((resolve) => setTimeout(resolve));

polyfill(document.body);
await nextFrame();
await settle();
log(`registered groups: ${state.m.size}, item observers: ${state.o.size}`);

const ownerA = document.querySelector("#a > [focusgroup]");
ownerA.remove();
await settle();
log(`removed owner A:          registered ${state.m.has(ownerA)}, groups ${state.m.size}, item observers ${state.o.size}`);

const ownerB = document.querySelector("#b > [focusgroup]");
document.getElementById("b").remove();
await settle();
log(`removed owner B's parent: registered ${state.m.has(ownerB)}, groups ${state.m.size}, item observers ${state.o.size}`);
</script>
Actual
registered groups: 2, item observers: 2
removed owner A:          registered false, groups 1, item observers 1
removed owner B's parent: registered true, groups 1, item observers 1
Cause

The global observer only disconnects a group when the removed node is its owner (src/polyfill.js#L38-L45):

for (const node of entry.removedNodes) {
  if (elementPolyfillMap.has(node)) {
    elementPolyfillMap.get(node)?.disconnect();
    elementPolyfillMap.delete(node);
  }
}

removedNodes lists only the root of the removed subtree, so owners inside it never match. elementPolyfillMap is a strong Map on the global state, and nothing else removes entries from it.

Workaround

I just create a new FocusGroup() each time, and call disconnect() manually.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start in packages/focusgroup/src/polyfill.js at the global observer handling around lines 38-45, then inspect how elementPolyfillMap and FocusGroup.disconnect() are used. Run the browser reproduction from the issue and verify that removing an ancestor unregisters the nested owner, disconnects its group, and removes its item observer without affecting other groups.

Written by the indexing model from the issue text.

Assessment

Tech stack
javascript
Domain
frontend
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
78/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.