fix(appkit-ui): stabilize useAnalyticsQuery params reference to avoid infinite refetch (#321)

* fix(appkit-ui): stabilize useAnalyticsQuery params reference

useAnalyticsQuery treated `parameters` as a useMemo dep by reference. A
typical call site passes a fresh object literal every render
(`useAnalyticsQuery("k", { limit: sql.number(10) })`), which invalidated
the payload memo and re-ran `start` -> infinite refetch. The workaround
was for every consumer to wrap params in `useMemo` — a recurring footgun.

Stabilize the params reference inside the hook using a useRef + shallow
structural equality check. Analytics query params are produced by the
`sql.*` builders and are always 1-level objects keyed to primitives, so
shallow equality is sufficient and substantially cheaper than a full
deep-equal. No public API change.

Also considered: stable-stringify + cache-keying on the string. Rejected
because params are tiny, structural equality is what users actually
want, and TanStack Query / SWR ship the same approach.

Updates the LLM guide which previously told consumers to always memoize
params.

Co-authored-by: Isaac
Signed-off-by: James Broadhead <jamesbroadhead@gmail.com>

* fix(appkit-ui): bump tsconfig lib from ES2021 to ES2022

The override added DOM lib but accidentally downleveled inherited
ES2022 from the root tsconfig, breaking Object.hasOwn (used in the
shallowEqualParams helper).

Signed-off-by: James Broadhead <jamesbroadhead@gmail.com>

* docs: drop obsolete useMemo guidance from llm-guide

The hook now stabilizes parameters internally, so the bullet was just
telling an LLM not to do something it wouldn't have done unprompted.

Co-authored-by: Isaac
Signed-off-by: James Broadhead <jamesbroadhead@gmail.com>

---------

Signed-off-by: James Broadhead <jamesbroadhead@gmail.com>

Author: jamesbroadhead

docs/docs/development/llm-guide.md

@@ -33,7 +33,6 @@ This guide is designed to work even when you *do not* have access to the AppKit
 
 - **Do not invent APIs**. If unsure, stick to the patterns shown in the documentation and only use documented exports from `@databricks/appkit` and `@databricks/appkit-ui`.
 - **`createApp()` is async**. Prefer **top-level `await createApp(...)`**. If you can't, use `void createApp(...)` and do not ignore promise rejection.
-- **Always memoize query parameters** passed to `useAnalyticsQuery` / charts to avoid refetch loops.
 - **Always handle loading/error/empty states** in UI (use `Skeleton`, error text, empty state).
 - **Always use `sql.*` helpers** for query parameters (do not pass raw strings/numbers unless the query expects none).
 - **Never construct SQL strings dynamically**. Use parameterized queries with `:paramName`.

packages/appkit-ui/src/react/hooks/__tests__/use-analytics-query.test.ts

@@ -0,0 +1,93 @@
+import { renderHook } from "@testing-library/react";
+import { afterEach, describe, expect, test, vi } from "vitest";
+
+// Mock connectSSE so the hook does not attempt a real network request.
+const mockConnectSSE = vi.fn().mockImplementation((_opts: unknown) => {
+  // Return a never-resolving promise; tests don't need the result.
+  return new Promise<void>(() => {});
+});
+
+vi.mock("@/js", () => ({
+  ArrowClient: {
+    fetchArrow: vi.fn(),
+    processArrowBuffer: vi.fn(),
+  },
+  connectSSE: (...args: unknown[]) => mockConnectSSE(...args),
+}));
+
+// Stub useQueryHMR so we don't pull in import.meta.hot wiring.
+vi.mock("../use-query-hmr", () => ({
+  useQueryHMR: () => {},
+}));
+
+import { useAnalyticsQuery } from "../use-analytics-query";
+
+describe("useAnalyticsQuery", () => {
+  afterEach(() => {
+    vi.clearAllMocks();
+  });
+
+  test("does not refetch when params object is structurally equal across renders", () => {
+    // Each render passes a fresh object literal — the common footgun.
+    const { rerender } = renderHook(
+      ({ limit }: { limit: number }) =>
+        // biome-ignore lint/suspicious/noExplicitAny: typed registry not available in tests
+        useAnalyticsQuery("test_query" as any, { limit } as any),
+      { initialProps: { limit: 10 } },
+    );
+
+    // Initial render triggers exactly one connection.
+    expect(mockConnectSSE).toHaveBeenCalledTimes(1);
+
+    // Re-render with structurally-equal-but-new-reference params.
+    rerender({ limit: 10 });
+    rerender({ limit: 10 });
+    rerender({ limit: 10 });
+
+    // Should NOT have refetched — the hook stabilized the params reference.
+    expect(mockConnectSSE).toHaveBeenCalledTimes(1);
+  });
+
+  test("does refetch when a param value actually changes", () => {
+    const { rerender } = renderHook(
+      ({ limit }: { limit: number }) =>
+        // biome-ignore lint/suspicious/noExplicitAny: typed registry not available in tests
+        useAnalyticsQuery("test_query" as any, { limit } as any),
+      { initialProps: { limit: 10 } },
+    );
+
+    expect(mockConnectSSE).toHaveBeenCalledTimes(1);
+
+    rerender({ limit: 20 });
+
+    expect(mockConnectSSE).toHaveBeenCalledTimes(2);
+  });
+
+  test("does not refetch when params is undefined across renders", () => {
+    const { rerender } = renderHook(() =>
+      // biome-ignore lint/suspicious/noExplicitAny: typed registry not available in tests
+      useAnalyticsQuery("test_query" as any),
+    );
+
+    expect(mockConnectSSE).toHaveBeenCalledTimes(1);
+
+    rerender();
+    rerender();
+
+    expect(mockConnectSSE).toHaveBeenCalledTimes(1);
+  });
+
+  test("treats two empty object literals as equal", () => {
+    const { rerender } = renderHook(() =>
+      // biome-ignore lint/suspicious/noExplicitAny: typed registry not available in tests
+      useAnalyticsQuery("test_query" as any, {} as any),
+    );
+
+    expect(mockConnectSSE).toHaveBeenCalledTimes(1);
+
+    rerender();
+    rerender();
+
+    expect(mockConnectSSE).toHaveBeenCalledTimes(1);
+  });
+});

packages/appkit-ui/src/react/hooks/use-analytics-query.ts

@@ -10,6 +10,55 @@ import type {
 } from "./types";
 import { useQueryHMR } from "./use-query-hmr";
 
+/**
+ * Shallow structural equality for analytics query parameter objects.
+ *
+ * Analytics query parameters are produced by the `sql.*` builders and are
+ * always plain objects keyed to primitive values (string | number | boolean
+ * | null | undefined), so shallow equality is sufficient and substantially
+ * cheaper than a full deep-equal.
+ */
+function shallowEqualParams(a: unknown, b: unknown): boolean {
+  if (Object.is(a, b)) return true;
+  if (
+    a === null ||
+    b === null ||
+    typeof a !== "object" ||
+    typeof b !== "object"
+  ) {
+    return false;
+  }
+  const aKeys = Object.keys(a as Record<string, unknown>);
+  const bKeys = Object.keys(b as Record<string, unknown>);
+  if (aKeys.length !== bKeys.length) return false;
+  for (const key of aKeys) {
+    if (!Object.hasOwn(b, key)) return false;
+    if (
+      !Object.is(
+        (a as Record<string, unknown>)[key],
+        (b as Record<string, unknown>)[key],
+      )
+    ) {
+      return false;
+    }
+  }
+  return true;
+}
+
+/**
+ * Stabilize a value's identity across renders when it is structurally equal
+ * to the previous value. Used to make object-literal parameters safe to pass
+ * directly to `useAnalyticsQuery` without forcing every consumer to wrap
+ * params in `useMemo`.
+ */
+function useStableParams<T>(value: T): T {
+  const ref = useRef<T>(value);
+  if (!shallowEqualParams(ref.current, value)) {
+    ref.current = value;
+  }
+  return ref.current;
+}
+
 function getDevMode() {
   const url = new URL(window.location.href);
   const searchParams = url.searchParams;
@@ -79,9 +128,18 @@ export function useAnalyticsQuery<
     );
   }
 
+  // Stabilize the parameters reference across renders. Without this, a fresh
+  // object literal at the call site (e.g. `useAnalyticsQuery("k", { limit: 10 })`)
+  // would change identity every render, invalidating the `payload` memo and
+  // re-running `start` -> infinite refetch loop.
+  const stableParameters = useStableParams(parameters);
+
   const payload = useMemo(() => {
     try {
-      const serialized = JSON.stringify({ parameters, format });
+      const serialized = JSON.stringify({
+        parameters: stableParameters,
+        format,
+      });
       const sizeInBytes = new Blob([serialized]).size;
       if (sizeInBytes > maxParametersSize) {
         throw new Error(
@@ -94,7 +152,7 @@ export function useAnalyticsQuery<
       console.error("useAnalyticsQuery: Failed to serialize parameters", error);
       return null;
     }
-  }, [parameters, format, maxParametersSize]);
+  }, [stableParameters, format, maxParametersSize]);
 
   const start = useCallback(() => {
     if (payload === null) {

packages/appkit-ui/tsconfig.json

@@ -3,7 +3,7 @@
   "compilerOptions": {
     "outDir": "dist",
     "jsx": "react-jsx",
-    "lib": ["ES2021", "DOM"],
+    "lib": ["ES2022", "DOM"],
     "baseUrl": ".",
     "paths": {
       "@/*": ["./src/*"],