fix(data-lit): dispose hooks on element disconnect

[kunalkindra]

Problem

Hooks from @adobe/data-lit (useEffect, useObservable, useObservableValues, useUpdated, useConnected, …) were never disposed when a Lit element unmounts. Every effect's cleanup was stored in host.hooks[i].dispose, but useEffect only invokes it on a dependency change — never on disconnect. As a result:

• Every useObservable / useObservableValues / useEffect subscription leaks when the element leaves the DOM, and keeps calling requestUpdate on a detached host.
• useConnected is inert: it listens for "connected" / "disconnected" events that nothing in the library ever dispatched.

This was never implemented (not a regression): no base element (ApplicationElement, DatabaseElement) ever overrode disconnectedCallback, so there was no disconnect edge at all. The only historical disconnectedCallback (ServiceApplication) disposed the whole service, not per-component hooks, and was removed long ago.

Fix

Install a single Lit ReactiveController from the withHooks render wrapper — the one seam all three opt-in patterns funnel through:

1. base-class constructor attachDecorator(this, 'render', withHooks) (ApplicationElement / DatabaseElement, incl. downstream forks),
2. the @withHooks method decorator on render(),
3. manual attachDecorator on plain LitElements.

The controller:

• hostDisconnected → dispatches "disconnected", then disposes and clears every hook slot (skipping value slots like useState/useRef), resetting the cursor.
• hostConnected → dispatches "connected"; on re-connect only, forces a requestUpdate() so hooks re-initialize and re-subscribe.

It's idempotent per host (double-wrapped renders install once) and feature-detects addController, so non-Lit hosts are safely skipped.

Zero consumer changes — enabling hooks now automatically enables cleanup. This also revives useConnected for free, and lets downstream repos retire hand-rolled ReactiveController workarounds for the missing disconnect edge.

Behavioral note

Disconnect is now treated as unmount (React-identical): on a DOM move (disconnect→reconnect of the same element), useState/useMemo reset and effects re-run, rather than persisting.

Testing

• New hooks-controller.test.ts (5 tests): disposes all effect hooks + resets cursor on disconnect (value/ref slots skipped, no crash); dispatches connected/disconnected; requestUpdate fires only on re-connect; idempotent install; non-reactive host is a no-op.
• pnpm test (10/10), pnpm typecheck, pnpm build all pass for @adobe/data-lit.

Also bumps the publishable surface to 0.10.8 via pnpm bump.

🤖 Generated with Claude Code

[krisnye]

Thanks for this @kunalkindra — this is a real, well-diagnosed leak and the fix targets exactly the right seam (withHooks), so it correctly covers all three opt-in paths including the service decorator in apply-service-decorators.ts. I traced first-mount / disconnect / reconnect ordering against use-effect.ts, use-connected.ts, use-state.ts, and stack.ts and the core logic is sound. Approving with comments.

Maintainer decisions

• Disconnect = full unmount (all hook state reset, including useState/useMemo/useRef, on any disconnect→reconnect): Approved as-is. The React-identical semantics are a predictable contract and the full-wipe is simpler and less error-prone than a positional "dispose effects but preserve value slots" scheme. The one condition: please document this as an explicit behavioral contract in the data-lit README (and/or CHANGELOG), not only in the PR description — a DOM move/reparent (el.remove() then re-append) silently discarding local UI state (toggles, inputs, scroll position) is a footgun consumers need to find in the docs, not by surprise.
• Real-DOM integration test: Requested in this PR if quick, otherwise an immediate follow-up — not a merge blocker (see Major initial enlistment #1).

Blockers

None.

Major

1. No real-Lit / real-DOM integration test. The suite drives a hand-rolled FakeHost extends EventTarget and manually calls connect()/disconnect(). That validates the controller's branching but does not exercise the actual bug path: that withHooks installs the controller, that a real element's addController fires hostConnected synchronously during render (which connectedOnce depends on), and that a real useObservable/useEffect subscription is genuinely torn down when the element leaves the DOM. I confirmed the package has no happy-dom/jsdom env configured, which explains the approach — but the highest-value regression test is missing: mount an element with useObservable, remove it, assert the observable has zero remaining subscribers and requestUpdate stops firing. Please add a DOM env + this one end-to-end test (in this PR if it's quick, otherwise as a fast follow-up).

2. Document the unmount semantics — see maintainer decision above. This is the one thing I'd like landed before merge.

Minor

1. Stray useState setter after disconnect. After disposeHooks sets host.hooks = [], a useState setter closure captured from the prior render still runs component.hooks[hookIndex] = newValue; requestUpdate() (use-state.ts:14), writing a stray value into the emptied array and calling requestUpdate on a detached host. It requires a subscription that wasn't torn down, so it's rare, but a one-line guard or comment in disposeHooks noting the assumption ("all live subscriptions have dispose and are cleared here") would help.

2. The "connected"/"disconnected" events are vestigial for internal consumers. At dispatch time on both first-mount and reconnect, no hook-registered listeners are present (first mount: not yet registered; reconnect: removed on the prior disconnect). useConnected works only via its direct isConnected check and its useEffect cleanup, not by receiving these events. Keeping them for external listeners is fine — just worth a code comment so future readers don't assume useConnected depends on the event path.

3. FakeHost timing fidelity. Its addController doesn't fire hostConnected synchronously the way real Lit does when already connected, so the "first connect happens at install" behavior that connectedOnce relies on is simulated by an explicit connect() rather than tested as it actually occurs. Noting for when the integration test above lands.

Nits

1. hooks-controller.ts — (host as Partial<ReactiveControllerHost>).addController: prefer 'addController' in host to drop the cast (it's a runtime capability check; the in narrowing reads cleaner).
2. Idempotency flag host[HOOKS_CONTROLLER] = true is set before addController; if addController threw, the flag would wrongly remain set. Harmless in practice, trivial to reorder.
3. Dispatches plain new Event("connected") rather than CustomEvent — fine for current listeners; only matters if detail/bubbling is ever wanted.
4. The PR body still says "0.10.8," now stale after the merge from main (diff bumps to 0.10.10). Worth correcting so the description matches.

Net: solid fix, no blockers. Land the docs note (major #2), add the DOM regression test (major #1, in-PR or fast follow-up), and the minors/nits are polish. 👍