FCE-3288 use current callback references (#526)

## Description

Introduces `useCurrentCallback` hook that ensures the closure values are
always latest.

## Motivation and Context

Invoking methods one after another in `useEffect` was prone to stale
closure issue.

## Documentation impact

- [ ] Documentation update required
- [ ] Documentation updated [in another PR](_)
- [x] No documentation update required

## Types of changes

- [x] Bug fix (non-breaking change which fixes an issue)
- [ ] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing
functionality to
      not work as expected)

Author: czerwiukk

packages/react-client/src/hooks/internal/useCurrentCallback.ts

@@ -0,0 +1,15 @@
+import { useCallback, useLayoutEffect, useRef } from "react";
+
+/**
+ * Returns a stable function reference whose body always reads the latest
+ * closure values. Use when a method needs to be called from effects or
+ * captured by consumers without being memoized against changing state
+ * (peerStatus, deviceTrack, etc.) in its dep array.
+ */
+export const useCurrentCallback = <Args extends unknown[], R>(handler: (...args: Args) => R) => {
+  const ref = useRef(handler);
+  useLayoutEffect(() => {
+    ref.current = handler;
+  });
+  return useCallback((...args: Args) => ref.current(...args), []);
+};

packages/react-client/src/hooks/internal/useScreenshareManager.ts

@@ -3,6 +3,7 @@ import { useCallback, useEffect, useRef, useState } from "react";
 
 import type { ScreenShareState } from "../../types/internal";
 import type { PeerStatus, TracksMiddleware } from "../../types/public";
+import { useCurrentCallback } from "./useCurrentCallback";
 
 export type UseScreenshareResult = {
   /**
@@ -160,7 +161,9 @@ export const useScreenShareManager = ({
     [state.stream, cleanMiddleware, replaceTracks],
   );
 
-  const stopStreaming: UseScreenshareResult["stopStreaming"] = useCallback(async () => {
+  // Stable identity with a live closure: peerStatus must be observed at call
+  // time so a captured reference doesn't skip the SFU removeTrack calls.
+  const stopStreaming: UseScreenshareResult["stopStreaming"] = useCurrentCallback(async () => {
     if (!state.stream) {
       logger.warn("No stream to stop");
       return;
@@ -180,22 +183,16 @@ export const useScreenShareManager = ({
 
     cleanMiddleware();
     setState((prev) => ({ stream: null, trackIds: null, tracksMiddleware: prev.tracksMiddleware }));
-  }, [
-    state.stream,
-    state.trackIds?.videoId,
-    state.trackIds?.audioId,
-    peerStatus,
-    cleanMiddleware,
-    logger,
-    fishjamClient,
-  ]);
+  });
 
   useEffect(() => {
     if (!state.stream) return;
     const [video, audio] = getTracksFromStream(state.stream);
 
     const trackEndedHandler = () => {
-      stopStreaming();
+      void stopStreaming().catch((err) => {
+        logger.error(err);
+      });
     };
 
     video.addEventListener("ended", trackEndedHandler);
@@ -205,20 +202,21 @@ export const useScreenShareManager = ({
       video.removeEventListener("ended", trackEndedHandler);
       audio?.removeEventListener("ended", trackEndedHandler);
     };
-  }, [state, stopStreaming]);
+  }, [state, stopStreaming, logger]);
 
   useEffect(() => {
     const onDisconnected = () => {
-      if (stream) {
-        stopStreaming();
-      }
+      if (!stream) return;
+      void stopStreaming().catch((err) => {
+        logger.error(err);
+      });
     };
     fishjamClient.on("disconnected", onDisconnected);
 
     return () => {
       fishjamClient.removeListener("disconnected", onDisconnected);
     };
-  }, [stopStreaming, fishjamClient, stream]);
+  }, [stopStreaming, fishjamClient, stream, logger]);
 
   return {
     startStreaming,

packages/react-client/src/hooks/internal/useTrackManager.ts

@@ -1,10 +1,11 @@
 import { type FishjamClient, type Logger, type TrackMetadata, TrackTypeError, Variant } from "@fishjam-cloud/ts-client";
-import { useCallback, useEffect, useRef } from "react";
+import { useEffect, useRef } from "react";
 
 import type { TrackManager } from "../../types/internal";
 import type { BandwidthLimits, PeerStatus, StreamConfig, TrackMiddleware } from "../../types/public";
 import { getConfigAndBandwidthFromProps, getRemoteOrLocalTrack } from "../../utils/track";
 import type { DeviceManager } from "./devices/useDeviceManager";
+import { useCurrentCallback } from "./useCurrentCallback";
 
 interface TrackManagerConfig {
   deviceManager: DeviceManager;
@@ -38,42 +39,40 @@ export const useTrackManager = ({
     selectDevice: _selectDevice,
   } = deviceManager;
 
-  const getCurrentTrackId = useCallback(() => {
+  // Read live deviceTrack from the `joined` listener without re-subscribing
+  // every time it changes.
+  const getDeviceTrack = useCurrentCallback(() => deviceTrack);
+
+  const getCurrentTrackId = (): string | null => {
     const refTrackId = currentTrackIdRef.current;
     if (!refTrackId) return null;
     const currentTrack = getRemoteOrLocalTrack(tsClient, refTrackId);
     return currentTrack?.trackId ?? null;
-  }, [tsClient]);
+  };
 
-  const selectDevice = useCallback(
-    async (deviceId: string) => {
-      const result = await _selectDevice(deviceId);
-      if (!result) return;
+  const selectDevice = useCurrentCallback(async (deviceId: string) => {
+    const result = await _selectDevice(deviceId);
+    if (!result) return;
 
-      const [newTrack, error] = result;
-      if (error) return error;
+    const [newTrack, error] = result;
+    if (error) return error;
 
-      const currentTrackId = getCurrentTrackId();
-      if (!currentTrackId) return;
+    const currentTrackId = getCurrentTrackId();
+    if (!currentTrackId) return;
 
-      await tsClient.replaceTrack(currentTrackId, newTrack);
-    },
-    [getCurrentTrackId, _selectDevice, tsClient],
-  );
+    await tsClient.replaceTrack(currentTrackId, newTrack);
+  });
 
-  const setTrackMiddleware = useCallback(
-    async (middleware: TrackMiddleware) => {
-      const processedTrack = await applyMiddleware(middleware);
+  const setTrackMiddleware = useCurrentCallback(async (middleware: TrackMiddleware) => {
+    const processedTrack = await applyMiddleware(middleware);
 
-      const currentTrackId = getCurrentTrackId();
-      if (!currentTrackId) return;
+    const currentTrackId = getCurrentTrackId();
+    if (!currentTrackId) return;
 
-      await tsClient.replaceTrack(currentTrackId, processedTrack);
-    },
-    [applyMiddleware, getCurrentTrackId, tsClient],
-  );
+    await tsClient.replaceTrack(currentTrackId, processedTrack);
+  });
 
-  const startStreaming = useCallback(
+  const startStreaming = useCurrentCallback(
     async (
       track: MediaStreamTrack,
       props: StreamConfig = { sentQualities: [Variant.VARIANT_LOW, Variant.VARIANT_MEDIUM, Variant.VARIANT_HIGH] },
@@ -102,31 +101,24 @@ export const useTrackManager = ({
         throw err;
       }
     },
-    [type, tsClient, bandwidthLimits, logger],
   );
 
-  const pauseStreaming = useCallback(
-    async (trackId: string) => {
-      if (peerStatus !== "connected") return;
-      await tsClient.replaceTrack(trackId, null);
-      return tsClient.updateTrackMetadata(trackId, { type, paused: true } satisfies TrackMetadata);
-    },
-    [tsClient, type, peerStatus],
-  );
+  const pauseStreaming = useCurrentCallback(async (trackId: string) => {
+    if (peerStatus !== "connected") return;
+    await tsClient.replaceTrack(trackId, null);
+    return tsClient.updateTrackMetadata(trackId, { type, paused: true } satisfies TrackMetadata);
+  });
 
-  const resumeStreaming = useCallback(
-    async (trackId: string, track: MediaStreamTrack) => {
-      if (peerStatus !== "connected") return;
-      await tsClient.replaceTrack(trackId, track);
-      return tsClient.updateTrackMetadata(trackId, { type, paused: false } satisfies TrackMetadata);
-    },
-    [peerStatus, tsClient, type],
-  );
+  const resumeStreaming = useCurrentCallback(async (trackId: string, track: MediaStreamTrack) => {
+    if (peerStatus !== "connected") return;
+    await tsClient.replaceTrack(trackId, track);
+    return tsClient.updateTrackMetadata(trackId, { type, paused: false } satisfies TrackMetadata);
+  });
 
   /**
    * @see {@link TrackManager#toggleMute} for more details.
    */
-  const toggleMute = useCallback(async () => {
+  const toggleMute = useCurrentCallback(async () => {
     const currentTrackId = getCurrentTrackId();
     const isTrackCurrentlyEnabled = Boolean(deviceTrack?.enabled);
     if (!currentTrackId) {
@@ -141,17 +133,17 @@ export const useTrackManager = ({
       enableDevice();
       await resumeStreaming(currentTrackId, deviceTrack);
     }
-  }, [deviceTrack, getCurrentTrackId, logger, disableDevice, pauseStreaming, enableDevice, resumeStreaming]);
+  });
 
   /**
    * @see {@link TrackManager#toggleDevice} for more details.
    */
-  const toggleDevice = useCallback(async () => {
+  const toggleDevice = useCurrentCallback(async () => {
     const currentTrackId = getCurrentTrackId();
     if (deviceTrack) {
       stopDevice();
       if (currentTrackId) {
-        pauseStreaming(currentTrackId);
+        await pauseStreaming(currentTrackId);
       }
     } else {
       const [newTrack, error] = await startDevice();
@@ -163,23 +155,18 @@ export const useTrackManager = ({
         await startStreaming(newTrack, streamConfig);
       }
     }
-  }, [
-    getCurrentTrackId,
-    deviceTrack,
-    stopDevice,
-    pauseStreaming,
-    startDevice,
-    peerStatus,
-    resumeStreaming,
-    startStreaming,
-    streamConfig,
-  ]);
+  });
 
   useEffect(() => {
     const onJoinedRoom = () => {
-      if (deviceTrack) {
-        startStreaming(deviceTrack, streamConfig);
-      }
+      const currentDeviceTrack = getDeviceTrack();
+      if (!currentDeviceTrack) return;
+      // The handler is sync; observe rejections so non-TrackTypeError failures
+      // from addTrack don't surface as unhandledrejection.
+      void startStreaming(currentDeviceTrack, streamConfig).catch((err) => {
+        if (err instanceof TrackTypeError) return;
+        logger.error(err);
+      });
     };
 
     const onLeftRoom = () => {
@@ -192,7 +179,7 @@ export const useTrackManager = ({
       tsClient.off("joined", onJoinedRoom);
       tsClient.off("disconnected", onLeftRoom);
     };
-  }, [deviceTrack, startStreaming, tsClient, streamConfig]);
+  }, [startStreaming, tsClient, streamConfig, getDeviceTrack, logger]);
 
   return {
     deviceTrack,

packages/react-client/src/hooks/useDataChannel.ts

@@ -4,6 +4,7 @@ import { useCallback, useContext, useEffect, useState } from "react";
 import { FishjamClientContext } from "../contexts/fishjamClient";
 import { PeerStatusContext } from "../contexts/peerStatus";
 import type { UseDataChannelResult } from "../types/public";
+import { useCurrentCallback } from "./internal/useCurrentCallback";
 
 /**
  * Hook for data channel operations - publish and subscribe to data.
@@ -49,7 +50,10 @@ export function useDataChannel(): UseDataChannelResult {
     };
   }, [client]);
 
-  const initialize = useCallback(async () => {
+  // Stable identity with a live closure: peerStatus / loading / ready must be
+  // observed at call time so a captured reference doesn't reject with a stale
+  // "Peer is not connected" right after the connect promise settles.
+  const initialize = useCurrentCallback(async () => {
     if (loading || ready) return;
 
     if (peerStatus !== "connected") {
@@ -67,7 +71,7 @@ export function useDataChannel(): UseDataChannelResult {
     } finally {
       setLoading(false);
     }
-  }, [client, peerStatus, loading, ready]);
+  });
 
   const publishData = useCallback(
     (data: Uint8Array, options: DataChannelOptions) => {

packages/react-client/src/tests/useCurrentCallback.spec.ts

@@ -0,0 +1,67 @@
+import { renderHook } from "@testing-library/react";
+import { describe, expect, it, vi } from "vitest";
+
+import { useCurrentCallback } from "../hooks/internal/useCurrentCallback";
+
+describe("useCurrentCallback", () => {
+  it("returns a stable function reference across re-renders", () => {
+    const { result, rerender } = renderHook((handler: () => void) => useCurrentCallback(handler), {
+      initialProps: () => undefined,
+    });
+
+    const firstRef = result.current;
+    rerender(() => undefined);
+    rerender(() => undefined);
+
+    expect(result.current).toBe(firstRef);
+  });
+
+  it("invokes the latest handler when called after a re-render", () => {
+    const first = vi.fn();
+    const second = vi.fn();
+
+    const { result, rerender } = renderHook((handler: () => void) => useCurrentCallback(handler), {
+      initialProps: first,
+    });
+
+    result.current();
+    expect(first).toHaveBeenCalledTimes(1);
+    expect(second).not.toHaveBeenCalled();
+
+    rerender(second);
+
+    result.current();
+    expect(first).toHaveBeenCalledTimes(1);
+    expect(second).toHaveBeenCalledTimes(1);
+  });
+
+  it("forwards arguments and returns the handler's value", () => {
+    const handler = vi.fn((a: number, b: number) => a + b);
+
+    const { result } = renderHook(() => useCurrentCallback(handler));
+
+    const sum = result.current(2, 3);
+
+    expect(handler).toHaveBeenCalledWith(2, 3);
+    expect(sum).toBe(5);
+  });
+
+  it("the wrapper captured before a re-render still calls the latest handler", () => {
+    const first = vi.fn(() => "first");
+    const second = vi.fn(() => "second");
+
+    const { result, rerender } = renderHook((handler: () => string) => useCurrentCallback(handler), {
+      initialProps: first,
+    });
+
+    // Simulates a consumer capturing the reference (e.g. inside a useEffect
+    // closure) before the handler closure has changed.
+    const captured = result.current;
+
+    rerender(second);
+
+    expect(captured()).toBe("second");
+    expect(second).toHaveBeenCalledTimes(1);
+    expect(first).not.toHaveBeenCalled();
+  });
+});