Fix data-lit state setter and resize render loops

packages/data-lit/README.md

@@ -16,6 +16,18 @@ import { ApplicationHost, DatabaseElement } from "@adobe/data-lit";
 import { createDatabase } from "@adobe/data/ecs";
 ```
 
+## State and resize callbacks
+
+`useState` returns a setter whose identity is stable for the lifetime of the hook state. The setter
+requests a render only when `Object.is` reports that the value has changed. Lazy initializers run once
+per hook state, including when the initial value is `null` or `undefined`.
+
+Treat state objects and arrays as immutable: pass a new object or array when its contents change.
+Mutating a value in place and passing the same reference to the setter does not request a render.
+
+`useResizeObserver` keeps its observer attached across renders and invokes the latest callback.
+Inline callbacks do not cause the observer to disconnect and reattach.
+
 ## Hooks and the disconnect lifecycle
 
 Hooks (`useState`, `useEffect`, `useObservable`, `useObservableValues`, `useMemo`, `useRef`, ...) follow

packages/data-lit/src/hooks/use-resize-observer.test.ts

@@ -0,0 +1,114 @@
+// @vitest-environment happy-dom
+import { afterEach, describe, it, expect, vi } from "vitest";
+import { LitElement, html } from "lit";
+import { attachDecorator } from "./attach-decorator.js";
+import { withHooks } from "./with-hooks.js";
+import { useMemo } from "./use-memo.js";
+import { useResizeObserver } from "./use-resize-observer.js";
+import { useState } from "./use-state.js";
+
+class ResizeProbeElement extends LitElement {
+    renders = 0;
+    offset = 0;
+    widths: number[] = [];
+    calculateCallback = vi.fn((setWidth: (value: number) => void) => setWidth);
+
+    constructor() {
+        super();
+        attachDecorator(this, "render", withHooks);
+    }
+
+    render() {
+        this.renders++;
+        const [width, setWidth] = useState(0);
+        const memoizedSetter = useMemo(() => this.calculateCallback(setWidth), [setWidth]);
+        const offset = this.offset;
+        useResizeObserver(info => {
+            this.widths.push(info.width + offset);
+            memoizedSetter(Math.floor(info.width + offset));
+        });
+        return html`<span>${width}</span>`;
+    }
+}
+customElements.define("resize-state-probe", ResizeProbeElement);
+
+function mockResizeObserver() {
+    const observers: { emit: (width: number) => void; observe: ReturnType<typeof vi.fn>; disconnect: ReturnType<typeof vi.fn> }[] = [];
+    vi.stubGlobal("ResizeObserver", class {
+        observe = vi.fn();
+        disconnect = vi.fn();
+
+        constructor(callback: ResizeObserverCallback) {
+            observers.push({
+                emit: width => callback([
+                    { contentRect: { width, height: 20 } } as ResizeObserverEntry,
+                ], this as unknown as ResizeObserver),
+                observe: this.observe,
+                disconnect: this.disconnect,
+            });
+        }
+    });
+    return observers;
+}
+
+afterEach(async () => {
+    document.body.replaceChildren();
+    await new Promise<void>(resolve => queueMicrotask(resolve));
+    vi.unstubAllGlobals();
+});
+
+describe("useResizeObserver with state", () => {
+    it("keeps one observer, uses the latest inline callback, and skips unchanged widths", async () => {
+        const observers = mockResizeObserver();
+        const element = document.createElement("resize-state-probe") as ResizeProbeElement;
+        document.body.appendChild(element);
+        await element.updateComplete;
+        const observer = observers[0];
+        expect(observer.observe).toHaveBeenCalledExactlyOnceWith(element);
+
+        observer.emit(100.2);
+        await element.updateComplete;
+        expect(element.shadowRoot?.textContent).toBe("100");
+        expect(element.renders).toBe(2);
+        expect(observers).toHaveLength(1);
+        expect(observer.disconnect).not.toHaveBeenCalled();
+        expect(element.calculateCallback).toHaveBeenCalledTimes(1);
+
+        const requestUpdate = vi.spyOn(element, "requestUpdate");
+        observer.emit(100.8);
+        await element.updateComplete;
+        expect(requestUpdate).not.toHaveBeenCalled();
+        expect(element.renders).toBe(2);
+
+        element.offset = 10;
+        element.requestUpdate();
+        await element.updateComplete;
+        observer.emit(200);
+        await element.updateComplete;
+        expect(element.widths).toEqual([100.2, 100.8, 210]);
+        expect(element.shadowRoot?.textContent).toBe("210");
+        expect(observers).toHaveLength(1);
+        expect(observer.disconnect).not.toHaveBeenCalled();
+        expect(element.calculateCallback).toHaveBeenCalledTimes(1);
+    });
+
+    it("disconnects on unmount and creates fresh state and observer on remount", async () => {
+        const observers = mockResizeObserver();
+        const element = document.createElement("resize-state-probe") as ResizeProbeElement;
+        document.body.appendChild(element);
+        await element.updateComplete;
+        observers[0].emit(100);
+        await element.updateComplete;
+
+        element.remove();
+        await new Promise<void>(resolve => queueMicrotask(resolve));
+        expect(observers[0].disconnect).toHaveBeenCalledTimes(1);
+
+        document.body.appendChild(element);
+        await element.updateComplete;
+        expect(element.shadowRoot?.textContent).toBe("0");
+        expect(observers).toHaveLength(2);
+        expect(observers[1].observe).toHaveBeenCalledExactlyOnceWith(element);
+        expect(element.calculateCallback).toHaveBeenCalledTimes(2);
+    });
+});

packages/data-lit/src/hooks/use-resize-observer.ts

@@ -2,6 +2,7 @@
 
 import { useEffect } from "./use-effect.js";
 import { useElement } from "./use-element.js";
+import { useRef } from "./use-ref.js";
 
 export interface ResizeInfo {
     width: number;
@@ -17,15 +18,17 @@ export interface ResizeInfo {
  */
 export function useResizeObserver(onResize: (info: ResizeInfo) => void) {
     const element = useElement();
+    const latestCallback = useRef(onResize);
+    latestCallback.current = onResize;
 
     useEffect(() => {
         const observer = new ResizeObserver(entries => {
             for (const entry of entries) {
                 const { width, height } = entry.contentRect;
-                onResize({ width, height, entry });
+                latestCallback.current({ width, height, entry });
             }
         });
         observer.observe(element);
         return () => observer.disconnect();
-    }, [element, onResize]);
+    }, [element]);
 } 

packages/data-lit/src/hooks/use-state.test.ts

@@ -0,0 +1,108 @@
+import { describe, it, expect, vi } from "vitest";
+import type { Component } from "./component/component.js";
+import { Component_stack } from "./component/stack.js";
+import { useState } from "./use-state.js";
+
+function createHost(): Component {
+    return Object.assign(new EventTarget(), {
+        isConnected: true,
+        hookIndex: 0,
+        hooks: [],
+        updatedListeners: new Set<() => void>(),
+        requestUpdate: vi.fn(),
+    });
+}
+
+function render<T>(host: Component, callback: () => T): T {
+    Component_stack.push(host);
+    try {
+        return callback();
+    } finally {
+        Component_stack.pop();
+    }
+}
+
+describe("useState", () => {
+    it("keeps the setter stable across renders and updates", () => {
+        const host = createHost();
+        const [, setValue] = render(host, () => useState(0));
+        expect(render(host, () => useState(0))[1]).toBe(setValue);
+
+        setValue(1);
+
+        const [value, nextSetter] = render(host, () => useState(0));
+        expect(value).toBe(1);
+        expect(nextSetter).toBe(setValue);
+        expect(host.requestUpdate).toHaveBeenCalledTimes(1);
+    });
+
+    it.each([0, NaN, null, undefined, "value", false, {}, []])(
+        "does not request an update for an Object.is-equal value: %s",
+        initial => {
+            const host = createHost();
+            const [, setValue] = render(host, () => useState(initial));
+
+            setValue(initial);
+            setValue(initial);
+
+            expect(host.requestUpdate).not.toHaveBeenCalled();
+        },
+    );
+
+    it("compares against the latest value before another render", () => {
+        const host = createHost();
+        const [, setValue] = render(host, () => useState(0));
+
+        setValue(1);
+        setValue(1);
+        setValue(2);
+
+        expect(host.requestUpdate).toHaveBeenCalledTimes(2);
+        expect(render(host, () => useState(0))[0]).toBe(2);
+    });
+
+    it("distinguishes positive and negative zero", () => {
+        const host = createHost();
+        const [, setValue] = render(host, () => useState(0));
+
+        setValue(-0);
+
+        expect(host.requestUpdate).toHaveBeenCalledTimes(1);
+        expect(render(host, () => useState(0))[0]).toBe(-0);
+    });
+
+    it.each([null, undefined])("initializes a nullish value only once: %s", initial => {
+        const host = createHost();
+        const initialize = vi.fn(() => initial);
+
+        render(host, () => useState(initialize));
+        render(host, () => useState(initialize));
+
+        expect(initialize).toHaveBeenCalledTimes(1);
+    });
+
+    it.each([null, undefined])("preserves a nullish update: %s", nextValue => {
+        const host = createHost();
+        const [, setValue] = render(host, () => useState<number | null | undefined>(1));
+
+        setValue(nextValue);
+
+        expect(render(host, () => useState(1))[0]).toBe(nextValue);
+    });
+
+    it("keeps state slots and hosts independent", () => {
+        const host = createHost();
+        const otherHost = createHost();
+        const read = () => [useState(0), useState(10)] as const;
+        const [first, second] = render(host, read);
+        const [other] = render(otherHost, read);
+
+        first[1](1);
+
+        expect(render(host, read).map(([value]) => value)).toEqual([1, 10]);
+        expect(render(otherHost, read).map(([value]) => value)).toEqual([0, 10]);
+        expect(first[1]).not.toBe(second[1]);
+        expect(first[1]).not.toBe(other[1]);
+        expect(otherHost.requestUpdate).not.toHaveBeenCalled();
+    });
+});

packages/data-lit/src/hooks/use-state.ts

@@ -2,18 +2,23 @@
 
 import { Component_stack } from "./component/stack.js";
 
+interface State<T> {
+    value: T;
+    setValue: (value: T) => void;
+}
+
 export function useState<T>(initializer: () => T): [T, (value: T) => void]
 export function useState<T>(initialValue: T): [T, (value: T) => void]
-export function useState<T>(initial: () => T) {
+export function useState<T>(initial: T | (() => T)): [T, (value: T) => void] {
     const component = Component_stack.active();
     const hookIndex = component.hookIndex++;
-    const value = component.hooks[hookIndex] ??= (typeof initial === "function" ? initial() : initial);
-    return [
-        value,
-        (newValue: T) => {
-            component.hooks[hookIndex] = newValue;
+    const state: State<T> = component.hooks[hookIndex] ??= {
+        value: typeof initial === "function" ? (initial as () => T)() : initial,
+        setValue: (newValue: T) => {
+            if (Object.is(state.value, newValue)) return;
+            state.value = newValue;
             component.requestUpdate();
-        }
-    ];
-
+        },
+    };
+    return [state.value, state.setValue];
 }