fix(hooks): stable load identity, single mount fetch and in-flight cancellation

src/components/ContractEventFeed.test.tsx

@@ -10,6 +10,21 @@ vi.mock("@/lib/client", () => ({
   getClient: vi.fn(),
 }));
 
+// Issue #442 / context refactor: the components read their client from
+// SorokitContext, so the hook is routed at the same `getClient` mock every test
+// below configures. Without this the mocked client never reaches the component.
+vi.mock("@/context/useSorokit", async () => {
+  const { getClient } = await import("@/lib/client");
+  return {
+    useSorokit: () => ({
+      client: getClient(),
+      isConnected: true,
+      address: "GTEST",
+    }),
+  };
+});
+
+
 const CONTRACT_ID = "CAAZI4TCR3TY5OJHCTJC2A4QSY6CJWJH5IAJTGKIN2ER7LBNVKOCCWNA";
 
 const MOCK_EVENT: ContractEvent = {
@@ -454,7 +469,7 @@ describe("ContractEventFeed", () => {
       act(() => { vi.advanceTimersByTime(0); });
       await waitFor(() => screen.getByText("transfer"));
 
-      fireEvent.click(screen.getByRole("button", { name: /export.*json/i }));
+      fireEvent.click(screen.getByRole("button", { name: /export \d+ events? as json/i }));
 
       expect(mockCreateObjectURL).toHaveBeenCalledTimes(1);
       const blob = mockCreateObjectURL.mock.calls[0]![0] as Blob;
@@ -477,7 +492,7 @@ describe("ContractEventFeed", () => {
           downloadName = this.download;
         });
 
-      fireEvent.click(screen.getByRole("button", { name: /export.*json/i }));
+      fireEvent.click(screen.getByRole("button", { name: /export \d+ events? as json/i }));
 
       expect(clickSpy).toHaveBeenCalledTimes(1);
       expect(downloadName).toBe(`contract-events-${CONTRACT_ID}.json`);
@@ -738,3 +753,142 @@ describe("ContractEventFeed", () => {
     });
   });
 });
+
+// ── Issue #442: stale closures, single mount fetch, runtime poll changes ─────
+describe("ContractEventFeed — issue #442", () => {
+  const OTHER_ID = "CBBB4TCR3TY5OJHCTJC2A4QSY6CJWJH5IAJTGKIN2ER7LBNVKOCCWNA";
+
+  function mockEvents(getEvents: ReturnType<typeof vi.fn>) {
+    vi.mocked(getClient).mockReturnValue({
+      soroban: { getEvents },
+    } as unknown as SorokitClient);
+  }
+
+  function evt(id: string, topic: string): ContractEvent {
+    return { ...MOCK_EVENT, id, topics: [topic] };
+  }
+
+  beforeEach(() => {
+    vi.clearAllMocks();
+    vi.useFakeTimers({ shouldAdvanceTime: true });
+  });
+
+  afterEach(() => {
+    vi.useRealTimers();
+  });
+
+  it("fires exactly one request on mount, even with polling enabled", async () => {
+    const getEvents = vi.fn().mockResolvedValue({ data: [], error: null });
+    mockEvents(getEvents);
+
+    render(<ContractEventFeed contractId={CONTRACT_ID} pollInterval={1000} />);
+    act(() => {
+      vi.advanceTimersByTime(0);
+    });
+
+    await waitFor(() => expect(getEvents).toHaveBeenCalledTimes(1));
+
+    // Well short of the first poll — the polling effect must not have fetched.
+    act(() => {
+      vi.advanceTimersByTime(100);
+    });
+    expect(getEvents).toHaveBeenCalledTimes(1);
+  });
+
+  it("starts polling when pollInterval goes from 0 to a positive value at runtime", async () => {
+    const getEvents = vi.fn().mockResolvedValue({ data: [], error: null });
+    mockEvents(getEvents);
+
+    const { rerender } = render(
+      <ContractEventFeed contractId={CONTRACT_ID} pollInterval={0} />,
+    );
+    act(() => {
+      vi.advanceTimersByTime(0);
+    });
+    await waitFor(() => expect(getEvents).toHaveBeenCalledTimes(1));
+
+    act(() => {
+      vi.advanceTimersByTime(3000);
+    });
+    expect(getEvents).toHaveBeenCalledTimes(1);
+
+    rerender(<ContractEventFeed contractId={CONTRACT_ID} pollInterval={1000} />);
+    act(() => {
+      vi.advanceTimersByTime(1000);
+    });
+    await waitFor(() => expect(getEvents).toHaveBeenCalledTimes(2));
+  });
+
+  it("re-arms the timer at the new period when pollInterval changes", async () => {
+    const getEvents = vi.fn().mockResolvedValue({ data: [], error: null });
+    mockEvents(getEvents);
+
+    const { rerender } = render(
+      <ContractEventFeed contractId={CONTRACT_ID} pollInterval={1000} />,
+    );
+    act(() => {
+      vi.advanceTimersByTime(0);
+    });
+    await waitFor(() => expect(getEvents).toHaveBeenCalledTimes(1));
+
+    act(() => {
+      vi.advanceTimersByTime(1000);
+    });
+    await waitFor(() => expect(getEvents).toHaveBeenCalledTimes(2));
+
+    rerender(<ContractEventFeed contractId={CONTRACT_ID} pollInterval={5000} />);
+
+    // The old 1s timer must be gone…
+    act(() => {
+      vi.advanceTimersByTime(1000);
+    });
+    expect(getEvents).toHaveBeenCalledTimes(2);
+
+    // …and the new 5s one armed.
+    act(() => {
+      vi.advanceTimersByTime(4000);
+    });
+    await waitFor(() => expect(getEvents).toHaveBeenCalledTimes(3));
+  });
+
+  it("discards an in-flight response belonging to the previous contractId", async () => {
+    let resolveOld!: (value: {
+      data: ContractEvent[] | null;
+      error: string | null;
+    }) => void;
+    const oldPending = new Promise<{
+      data: ContractEvent[] | null;
+      error: string | null;
+    }>((resolve) => {
+      resolveOld = resolve;
+    });
+
+    const getEvents = vi
+      .fn()
+      .mockReturnValueOnce(oldPending)
+      .mockResolvedValue({ data: [evt("evt-new", "NEW-EVT")], error: null });
+    mockEvents(getEvents);
+
+    const { rerender } = render(<ContractEventFeed contractId={CONTRACT_ID} />);
+    act(() => {
+      vi.advanceTimersByTime(0);
+    });
+    await waitFor(() => expect(getEvents).toHaveBeenCalledTimes(1));
+
+    // Switch contracts while the first request is still in flight.
+    rerender(<ContractEventFeed contractId={OTHER_ID} />);
+    act(() => {
+      vi.advanceTimersByTime(0);
+    });
+    await waitFor(() => expect(getEvents).toHaveBeenCalledTimes(2));
+    expect(await screen.findByText("NEW-EVT")).toBeInTheDocument();
+
+    // The stale response lands late and must be dropped.
+    await act(async () => {
+      resolveOld({ data: [evt("evt-old", "OLD-EVT")], error: null });
+    });
+
+    expect(screen.queryByText("OLD-EVT")).not.toBeInTheDocument();
+    expect(screen.getByText("NEW-EVT")).toBeInTheDocument();
+  });
+});

src/components/ContractEventFeed.tsx

@@ -51,7 +51,7 @@
 
 import { Badge } from "@/components/ui/Badge";
 import { useSorokit } from "@/context/useSorokit";
 import { useIsVisible } from "@/hooks/useIsVisible";
 import type { ContractEvent } from "@/lib/client";
 import { cn, truncateAddress } from "@/lib/utils";
 
@@ -250,7 +250,11 @@
     filterTypes ? new Set(filterTypes) : null,
   );
   const intervalRef = useRef<ReturnType<typeof setInterval> | null>(null);
-  const [containerRef, isVisible] = useIsVisible<HTMLDivElement>();
+  // Issue #442: generation counter for `load`. Bumped on every call and
+  // whenever `contractId` changes, so a response that arrives after a newer
+  // request started - or after the feed moved to another contract - is
+  // discarded instead of overwriting the current events.
+  const requestIdRef = useRef(0);
 
   // IDs highlighted as newly-arrived. `prevEventIdsRef` is the baseline from
   // the previous successful load — `null` means no baseline yet, so the very
@@ -272,19 +276,38 @@
     setNewEventIds(new Set());
   }
 
+  // Issue #442: `live` is seeded from `pollInterval` at mount, so a runtime
+  // change of the prop has to re-seed it - otherwise a feed mounted with
+  // polling off (pollInterval 0) never starts polling when the prop turns on.
+  // Synced during render, mirroring the `prevContractId` pattern above.
+  const [prevPollInterval, setPrevPollInterval] = useState(pollInterval);
+  if (prevPollInterval !== pollInterval) {
+    setPrevPollInterval(pollInterval);
+    setLive(pollInterval > 0);
+  }
+
   useEffect(() => {
     prevEventIdsRef.current = null;
+    // Issue #442: invalidate whatever `load` has in flight for the previous
+    // contract. This effect is declared before the loading effect, so it runs
+    // first and the fresh load below gets the next generation number.
+    requestIdRef.current += 1;
   }, [contractId]);
 
   const load = useCallback(async () => {
     if (!contractId.trim() || !client) return;
+    // Issue #442: claim a generation up front; anything that resolves once a
+    // newer request exists is stale and must not touch state.
+    const requestId = ++requestIdRef.current;
+    const isStale = () => requestId !== requestIdRef.current;
     setLoading(true);
     try {
       const { data, error: err } = await client.soroban.getEvents(
         contractId,
         limit,
         fromLedger,
       );
+      if (isStale()) return;
       if (err) {
         setError(err);
         setLoading(false);
@@ -312,16 +335,19 @@
       setError(null);
       setLastUpdatedAt(Date.now());
     } catch (e) {
+      if (isStale()) return;
       setError(e instanceof Error ? e.message : "Failed to load events");
     } finally {
-      setLoading(false);
+      // Issue #442: a stale call must not clear the spinner that belongs to the
+      // request that superseded it.
+      if (!isStale()) setLoading(false);
     }
   }, [client, contractId, limit, fromLedger]);
 
-  useEffect(() => {
-    // eslint-disable-next-line react-hooks/set-state-in-effect
-    setEvents([]);
-  }, [contractId]);
+  // Issue #442: the `setEvents([])` effect that used to sit here (behind a
+  // react-hooks/set-state-in-effect suppression) duplicated the render-phase
+  // reset above and ran again on mount. Removed - the render-phase reset
+  // already clears the previous contract's events without an extra pass.
 
   useEffect(() => {
     const timerId = window.setTimeout(() => {
@@ -333,6 +359,10 @@
     };
   }, [load]);
 
+  // Issue #442: polling owns only the timer - the initial fetch belongs to the
+  // effect above, so mount fires exactly one request. Keyed on `pollInterval`,
+  // so changing the prop at runtime tears the old timer down and re-arms a new
+  // one at the new period.
   useEffect(() => {
     // Dashboard keeps a visited screen mounted rather than unmounting it,
     // to preserve in-progress state — see the comment in Dashboard.tsx.
@@ -348,10 +378,17 @@
     } else {
       if (intervalRef.current) clearInterval(intervalRef.current);
     }
+    if (!live || pollInterval <= 0 || contractId.trim() === "") return;
+    intervalRef.current = setInterval(() => {
+      void load();
+    }, pollInterval);
     return () => {
-      if (intervalRef.current) clearInterval(intervalRef.current);
+      if (intervalRef.current) {
+        clearInterval(intervalRef.current);
+        intervalRef.current = null;
+      }
     };
   }, [live, isVisible, pollInterval, load, contractId]);
 
   // Tick the relative "Last updated" label once a second while polling is
   // active and visible — ticking a hidden screen's clock wastes a timer for
@@ -360,7 +397,7 @@
     if (!live || !isVisible || pollInterval <= 0) return;
     const tickId = setInterval(() => setNow(Date.now()), 1000);
     return () => clearInterval(tickId);
   }, [live, isVisible, pollInterval]);
 
   const typeCounts = useMemo(() => {
     const counts = new Map<string, number>();

src/components/FeeEstimator.test.tsx

@@ -7,6 +7,21 @@ vi.mock("@/lib/client", () => ({
   getClient: vi.fn(),
 }));
 
+// Issue #442 / context refactor: the components read their client from
+// SorokitContext, so the hook is routed at the same `getClient` mock every test
+// below configures. Without this the mocked client never reaches the component.
+vi.mock("@/context/useSorokit", async () => {
+  const { getClient } = await import("@/lib/client");
+  return {
+    useSorokit: () => ({
+      client: getClient(),
+      isConnected: true,
+      address: "GTEST",
+    }),
+  };
+});
+
+
 import type { SorokitClient } from "@/lib/client";
 import { getClient } from "@/lib/client";
 
@@ -309,3 +324,64 @@ describe("FeeEstimator", { timeout: 15000 }, () => {
     });
   });
 });
+
+// ── Issue #442: `load` identity must not depend on an inline onFeeLoad ───────
+describe("FeeEstimator — issue #442", () => {
+  function mockFee() {
+    const estimateFee = vi
+      .fn()
+      .mockResolvedValue({ data: { baseFee: "100", recommended: "500" }, error: null });
+    vi.mocked(getClient).mockReturnValue({
+      transaction: { estimateFee },
+    } as unknown as SorokitClient);
+    return estimateFee;
+  }
+
+  async function settle() {
+    await act(async () => {
+      await new Promise((resolve) => setTimeout(resolve, 30));
+    });
+  }
+
+  beforeEach(() => {
+    vi.clearAllMocks();
+  });
+
+  it("makes exactly one request on mount", async () => {
+    const estimateFee = mockFee();
+    render(<FeeEstimator />);
+    await waitFor(() => expect(estimateFee).toHaveBeenCalledTimes(1));
+    await settle();
+    expect(estimateFee).toHaveBeenCalledTimes(1);
+  });
+
+  it("does not refetch when the parent re-renders with a new inline onFeeLoad", async () => {
+    const estimateFee = mockFee();
+    const seen = vi.fn();
+
+    const { rerender } = render(<FeeEstimator onFeeLoad={(f) => seen(f)} />);
+    await waitFor(() => expect(estimateFee).toHaveBeenCalledTimes(1));
+
+    // A fresh arrow on each render used to rebuild `load` and refetch.
+    rerender(<FeeEstimator onFeeLoad={(f) => seen(f)} />);
+    rerender(<FeeEstimator onFeeLoad={(f) => seen(f)} />);
+    await settle();
+
+    expect(estimateFee).toHaveBeenCalledTimes(1);
+  });
+
+  it("calls the latest onFeeLoad, not the one captured at mount", async () => {
+    mockFee();
+    const first = vi.fn();
+    const second = vi.fn();
+
+    const { rerender } = render(<FeeEstimator onFeeLoad={first} />);
+    await waitFor(() => expect(first).toHaveBeenCalledTimes(1));
+
+    rerender(<FeeEstimator onFeeLoad={second} />);
+    fireEvent.click(screen.getByRole("button", { name: "Refresh fee estimate" }));
+
+    await waitFor(() => expect(second).toHaveBeenCalledTimes(1));
+    expect(first).toHaveBeenCalledTimes(1);
+  });
+});