[labs/signals] SignalWatcher elements are never garbage collected (FinalizationRegistry heldValue + elementForWatcher WeakMap cycle)

Which package(s) are affected? @lit-labs/signals (0.3.0, and the code is unchanged on main)

Description

Elements using the SignalWatcher mixin are never garbage collected after being removed from the DOM. The module-level bookkeeping in signal-watcher.ts forms a cycle that GC cannot break:

// packages/labs/signals/src/lib/signal-watcher.ts
elementForWatcher.set(watcher, this as unknown as SignalWatcherInternal);
elementFinalizationRegistry.register(this, watcher);

The FinalizationRegistry holds its heldValue (the watcher) strongly until the registered target (the element) is collected. Meanwhile elementForWatcher is a WeakMap<Watcher, Element> — the key is weak, but the watcher is kept alive by the registry, so the map entry survives and its value keeps the element strongly reachable. So the element can't be collected until the watcher is released, and the watcher can't be released until the element is collected. Neither ever happens.

The cleanup in disconnectedCallback → unwatchSignals drops the element's own references to the watcher, but never unregisters from the FinalizationRegistry, so the cycle above is unaffected.

We found this hunting a leak in our app: heap snapshots showed every disconnected SignalWatcher host retained, and the only strong retainer path went through the elementForWatcher WeakMap entry (key: the watcher held by the registry, value: the element). Once we patched the package as described below, all of them collected.

Reproduction

import {LitElement, html} from 'lit';
import {SignalWatcher, signal} from '@lit-labs/signals';

const count = signal(0);

class LeakyElement extends SignalWatcher(LitElement) {
  render() {
    return html`${count.get()}`;
  }
}
customElements.define('leaky-element', LeakyElement);

const refs = [];
for (let i = 0; i < 50; i++) {
  const el = document.createElement('leaky-element');
  document.body.append(el);
  await el.updateComplete;
  el.remove();
  refs.push(new WeakRef(el));
}

// wait a macrotask so the disconnectedCallback microtask cleanup runs,
// then force GC (DevTools "Collect garbage" or a heap snapshot)
setTimeout(() => {
  console.log(refs.filter((r) => r.deref() !== undefined).length); // 50, expected 0
}, 1000);

Expected behavior

A disconnected, otherwise unreferenced element becomes collectible; the FinalizationRegistry callback then unwatches its signals.

Actual behavior

All 50 elements stay reachable forever. In a long-lived SPA this means every SignalWatcher component instance ever rendered is retained, along with its shadow tree and anything it references.

Suggested fix

Store a WeakRef in the map and pass an unregister token:

elementForWatcher.set(watcher, new WeakRef(this));
elementFinalizationRegistry.register(this, watcher, this);

with elementForWatcher.get(this)?.deref() at the read site in the watcher callback (the el === undefined early-return already handles the collected case), and elementFinalizationRegistry.unregister(this) in the full-teardown branch of unwatchSignals so a cleanly disconnected element doesn't linger in the registry.

We've been running exactly this as a patch-package patch and it resolves the leak with no behavior change (existing test suite still green on our side). Happy to turn it into a PR if that's welcome.

Possibly related: #4735 reports a similar symptom in @lit-labs/preact-signals, but the mechanism there looks different.

Version: @lit-labs/[email protected], [email protected], Chrome 138 (macOS). Reproduces in any browser with FinalizationRegistry semantics per spec.