Fix pending output observer callbacks after unbind

branch-review-findings.md

@@ -0,0 +1,73 @@
+# Branch Review Findings
+
+Review date: 2026-04-21
+Branch: `resize-observer-v2`
+Reviewer: Codex
+
+## Status
+
+- [ ] Finding 1: Restore the guarantee that output clientdata is flushed before the next input batch is sent
+- [x] Finding 2: Prevent post-unbind observer callbacks from emitting `.clientdata_output_null_*` inputs
+- [ ] Simplification: Move output-info observer setup / flush / disposal behind a dedicated module API
+
+## Finding 1
+
+Severity: High
+Status: Open
+
+Summary:
+The new observer path no longer guarantees that output clientdata is flushed before the next input batch is sent.
+
+Details:
+The central `sendOutputInfoFns` debouncer is still wired into `InputBatchSender.lastChanceCallback`, but the per-element `ResizeObserver` and `IntersectionObserver` handlers in `srcts/src/shiny/index.ts` debounce for 100ms and then call `doSendHiddenState()` / `doSendSize()` directly, bypassing that flush path.
+
+Risk:
+A tab/show/layout change followed immediately by a click, brush, or other input can reach the server with stale `.clientdata_output_*` width, height, or hidden values.
+
+Relevant files:
+- `srcts/src/shiny/index.ts`
+- `srcts/src/shiny/sendOutputInfo.ts`
+
+Notes:
+- Old behavior routed these updates through debouncers that were explicitly flushed before send.
+
+## Finding 2
+
+Severity: Medium
+Status: Fixed on branch
+
+Summary:
+Queued observer callbacks can survive unbind and emit bogus `.clientdata_output_null_*` inputs.
+
+Details:
+`getIdFromEl()` now returns `null` when `shiny-output-binding` has already been removed, but the callbacks created in `ensureObservers()` have no cancellation mechanism. `unbindOutputs()` disconnects the observers, but it does not cancel already-scheduled debounced callbacks; when one fires after unbind, `doSendHiddenState()` / `doSendSize()` / `doSendTheme()` will build keys from `id === null`.
+
+Risk:
+Dynamic UI replacement or removal can send invalid clientdata inputs after the output has already been unbound.
+
+Relevant files:
+- `srcts/src/shiny/index.ts`
+- `srcts/src/shiny/bind.ts`
+- `srcts/src/time/debounce.ts`
+
+Resolution:
+Added explicit cancellation support to the shared `debounce()` helper, stored the debounced observer callbacks alongside each observer, and canceled those callbacks during `unbindOutputs()` before removing the output binding data.
+
+## Simplification
+
+Status: Open
+
+Summary:
+Most of the new output-info state machine still lives inline in `initialize()`.
+
+Opportunity:
+Pull observer setup, flush behavior, and disposal behind one module-owned API so that lifecycle handling is centralized and easier to reason about.
+
+Relevant files:
+- `srcts/src/shiny/index.ts`
+- `srcts/src/shiny/sendOutputInfo.ts`
+
+## Verification
+
+- `npm run build_types` passed during review
+- Browser-level regression coverage for this new output-info pipeline was not run

srcts/src/shiny/bind.ts

@@ -442,6 +442,17 @@ function unbindOutputs(
       }
     }
 
+    for (const key of [
+      "shiny-resize-observer-callback",
+      "shiny-intersection-observer-callback",
+      "shiny-mutate-observer-callback",
+    ]) {
+      const callback = $el.data(key) as { cancel?: () => void } | undefined;
+
+      callback?.cancel?.();
+      $el.removeData(key);
+    }
+
     $el.trigger({
       type: "shiny:unbound",
       // @ts-expect-error; Can not remove info on a established, malformed Event object

srcts/src/shiny/index.ts

@@ -433,6 +433,7 @@ class ShinyClass {
         const ro = new ResizeObserver(() => onResize());
 
         ro.observe(el);
+        $(el).data("shiny-resize-observer-callback", onResize);
         $(el).data("shiny-resize-observer", ro);
       }
 
@@ -447,6 +448,7 @@ class ShinyClass {
         const io = new IntersectionObserver(() => onIntersect());
 
         io.observe(el);
+        $(el).data("shiny-intersection-observer-callback", onIntersect);
         $(el).data("shiny-intersection-observer", io);
       }
 
@@ -464,6 +466,7 @@ class ShinyClass {
         });
 
         $(el).data("shiny-mutate-observer", mo);
+        $(el).data("shiny-mutate-observer-callback", onMutate);
       }
     }
 

srcts/src/time/__tests__/debounce.test.ts

@@ -0,0 +1,18 @@
+import test from "node:test";
+import assert from "node:assert/strict";
+
+import { debounce } from "../debounce";
+
+test("debounce can cancel a pending callback before it fires", async () => {
+  let calls = 0;
+  const debounced = debounce(10, () => {
+    calls += 1;
+  }) as ReturnType<typeof debounce> & { cancel?: () => void };
+
+  debounced();
+  debounced.cancel?.();
+
+  await new Promise((resolve) => setTimeout(resolve, 30));
+
+  assert.equal(calls, 0);
+});

srcts/src/time/debounce.ts

@@ -70,15 +70,21 @@ class Debouncer<X extends AnyVoidFunction> implements InputRatePolicy<X> {
 // 900ms intervals will result in a single execution
 // of the underlying function, 1000ms after the 17th
 // call.
+type DebouncedFunction<T extends (...args: unknown[]) => void> = ((
+  ...args: Parameters<T>
+) => void) & {
+  cancel: () => void;
+};
+
 function debounce<T extends (...args: unknown[]) => void>(
   threshold: number | undefined,
   func: T,
-): (...args: Parameters<T>) => void {
+): DebouncedFunction<T> {
   let timerId: ReturnType<typeof setTimeout> | null = null;
 
   // Do not alter `function()` into an arrow function.
   // The `this` context needs to be dynamically bound
-  return function thisFunc(...args: Parameters<T>) {
+  const debounced = function thisFunc(...args: Parameters<T>) {
     if (timerId !== null) {
       clearTimeout(timerId);
       timerId = null;
@@ -92,6 +98,16 @@ function debounce<T extends (...args: unknown[]) => void>(
       func.apply(thisFunc, args);
     }, threshold);
   };
+
+  debounced.cancel = function () {
+    if (timerId !== null) {
+      clearTimeout(timerId);
+      timerId = null;
+    }
+  };
+
+  return debounced;
 }
 
 export { debounce, Debouncer };
+export type { DebouncedFunction };

srcts/types/src/time/debounce.d.ts

@@ -14,5 +14,9 @@ declare class Debouncer<X extends AnyVoidFunction> implements InputRatePolicy<X>
     $clearTimer(): void;
     $invoke(): void;
 }
-declare function debounce<T extends (...args: unknown[]) => void>(threshold: number | undefined, func: T): (...args: Parameters<T>) => void;
+type DebouncedFunction<T extends (...args: unknown[]) => void> = ((...args: Parameters<T>) => void) & {
+    cancel: () => void;
+};
+declare function debounce<T extends (...args: unknown[]) => void>(threshold: number | undefined, func: T): DebouncedFunction<T>;
 export { debounce, Debouncer };
+export type { DebouncedFunction };