Skip to content

feat(react): Deprecate usePersistedDevicePreferences and move the logic to the SDK core#2108

Merged
oliverlaz merged 26 commits intomainfrom
device-persistence
Feb 25, 2026
Merged

feat(react): Deprecate usePersistedDevicePreferences and move the logic to the SDK core#2108
oliverlaz merged 26 commits intomainfrom
device-persistence

Conversation

@oliverlaz
Copy link
Copy Markdown
Member

@oliverlaz oliverlaz commented Feb 4, 2026

💡 Overview

Deprecates usePersistedDevicePreferences hook as it didn't work well in all cases.
From now on, the device persistence will be handled by our core SDK and will be enabled by default.

We don't remove the usePersistedDevicePreferences from the SDK yet, as that would be a breaking change. Its implementation is a no-op now, and we log a warning, asking integrators to remove it from their codebase.

🎫 Ticket: https://linear.app/stream/issue/REACT-809/move-device-persistence-logic-to-the-client-package
📑 Docs: https://github.com/GetStream/docs-content/pull/993

Summary by CodeRabbit

  • New Features

    • Web device persistence: camera, microphone, and speaker selections plus mute state are saved and restored across sessions and can influence initial device behavior.
  • Chores

    • Old persisted-device hook deprecated and replaced with a no-op that emits a deprecation warning directing users to the new device preferences flow.
  • Other

    • Sample app now enables device persistence by default; comprehensive tests added for persistence and device handling.

@oliverlaz oliverlaz requested a review from jdimovska February 4, 2026 16:08
@changeset-bot
Copy link
Copy Markdown

changeset-bot bot commented Feb 4, 2026

⚠️ No Changeset found

Latest commit: 9b657ad

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Feb 4, 2026

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Adds web device-preference persistence: new devicePersistence module; StreamClient/Call accept and normalize devicePersistence options and pass them into DeviceManager and concrete managers; DeviceManager and Camera/Microphone/Speaker managers persist and apply preferences via localStorage; tests, mocks, sample app, and a React hook updated.

Changes

Cohort / File(s) Summary
Device persistence core
packages/client/src/devices/devicePersistence.ts
New module: DevicePersistenceOptions types, normalize(), readPreferences(), writePreferences(), defaultDeviceId, createSyntheticDevice(), toPreferenceList(); 3-entry deduping history and localStorage handling.
Device manager base
packages/client/src/devices/DeviceManager.ts
Constructor now accepts devicePersistence; adds devicePersistence field, persist/apply helpers (persistPreference, applyPersistedPreferences, applyMutedState), subscription callbacks array, and wiring to persist on state changes.
Camera / Microphone / Speaker / ScreenShare
packages/client/src/devices/CameraManager.ts, .../MicrophoneManager.ts, .../SpeakerManager.ts, .../ScreenShareManager.ts
Constructors updated to accept normalized devicePersistence and forward to base; Camera/Microphone apply flows now consider persisted preferences vs defaults; SpeakerManager adds web vs RN apply paths and speaker preference persistence; ScreenShare passes default normalized persistence (disabled) to base.
Client types & wiring
packages/client/src/Call.ts, packages/client/src/coordinator/connection/types.ts, packages/client/src/types.ts
Added devicePersistence?: DevicePersistenceOptions to StreamClient/Call option types; Call now normalizes and passes devicePersistence into device managers; moved one import in types.ts.
Device streams & permissions
packages/client/src/devices/devices.ts, packages/client/src/permissions/PermissionsContext.ts
Device enumerables start with empty arrays (startWith([])); PermissionsContext.canPublish returns explicit false for unknown track types.
Tests, mocks & helpers
packages/client/src/devices/__tests__/*, packages/client/src/devices/__tests__/mocks.ts, .../mediaStreamTestHelpers.ts
Tests updated/added for persistence (Camera/Microphone/Speaker/DeviceManager); LocalStorage mock added; media stream test helpers added; many tests now pass devicePersistence into managers.
React SDK & sample app
packages/react-sdk/src/hooks/usePersistedDevicePreferences.ts, sample-apps/react/react-dogfood/*
usePersistedDevicePreferences replaced with a no-op that warns of deprecation; sample app adds devicePersistence option via isRecentDeviceSelectionEnabled helper; device-selection hook updated for new key semantics.
Misc & small changes
packages/client/src/helpers/__tests__/DynascaleManager.test.ts, others
Call/StreamClient test usages updated to include devicePersistence in options; small import/cleanup and test adjustments across suites.

Sequence Diagram(s)

sequenceDiagram
    participant App as Application
    participant Client as StreamClient/Call
    participant DeviceMgr as DeviceManager (Camera/Mic/Speaker)
    participant Storage as localStorage

    Note over App,Client: Init with devicePersistence config
    App->>Client: new StreamClient({ devicePersistence })
    Client->>DeviceMgr: new CameraManager(call, normalizedPersistence)

    Note over App,DeviceMgr: User selects or mutes device
    App->>DeviceMgr: selectDevice(deviceId)
    DeviceMgr->>DeviceMgr: persistPreference(deviceId, status)
    DeviceMgr->>Storage: writePreferences(storageKey, preference)

    Note over Client,DeviceMgr: New session / initialization
    App->>Client: new StreamClient({ devicePersistence })
    Client->>DeviceMgr: new MicrophoneManager(call, normalizedPersistence)
    DeviceMgr->>Storage: readPreferences(storageKey)
    Storage-->>DeviceMgr: stored preferences
    DeviceMgr->>DeviceMgr: applyPersistedPreferences()
    DeviceMgr->>DeviceMgr: selectDevice(restoredDeviceId)
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Poem

🐰 I hop through keys and tiny IDs tonight,

I tuck preferences safe where localStorage sleeps,
Cameras, mics and speakers find their light,
Restored at dawn when every app reawakens,
A rabbit's promise: settings kept and neat.

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main change: deprecating a React hook and moving its logic to the SDK core.
Description check ✅ Passed The description covers the overview and includes references to the ticket and docs PR as required by the template, though implementation notes are minimal.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch device-persistence

Tip

Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs).
Share your feedback on Discord.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
packages/client/src/devices/DeviceManager.ts (1)

75-96: ⚠️ Potential issue | 🟠 Major

Limit persistence subscriptions to audio/video managers.

persistPreference() relies on mediaDeviceKind, which throws for non-audio/video track types. Since persistence is enabled by default, screen-share managers will hit this path and crash. Guard the subscription to audio/video only.

🐛 Suggested fix
-    if (this.devicePersistence.enabled) {
+    if (
+      this.devicePersistence.enabled &&
+      (this.trackType === TrackType.AUDIO || this.trackType === TrackType.VIDEO)
+    ) {
       this.subscriptions.push(
         createSubscription(
           combineLatest([this.state.selectedDevice$, this.state.status$]),
           ([selectedDevice, status]) => {
             this.persistPreference(selectedDevice, status);
           },
         ),
       );
     }
🤖 Fix all issues with AI agents
In `@packages/client/src/devices/DeviceManager.ts`:
- Around line 600-639: The function applyPersistedPreferences should
early-return when device persistence is disabled or when the track type is
neither audio nor video: add a short-circuit at the start of
applyPersistedPreferences that checks this.devicePersistence.enabled (or
equivalent flag) and this.trackType being TrackType.AUDIO or TrackType.VIDEO,
and return false if persistence is disabled or the trackType is not audio/video;
keep the rest of the logic (DevicePreferenceKey selection, readPreferences,
toPreferenceList, calling select and applyMutedState) unchanged so you only
avoid applying stored preferences in unsupported cases.

In `@packages/client/src/devices/devicePersistence.ts`:
- Around line 51-91: The writePreferences function assumes window.localStorage
is always available; wrap any localStorage access in a runtime guard and
best-effort try/catch so failures (SSR, React Native, private mode, quota
errors) don’t crash. Concretely, in writePreferences (and optionally
readPreferences) check typeof window !== 'undefined' and typeof
window.localStorage !== 'undefined' before calling setItem, and surround setItem
with a try/catch that swallows or logs the error without throwing; keep the rest
of the logic (selectedDeviceId/selectedDeviceLabel, building nextPreferences
using DevicePreferenceKey, LocalDevicePreferences and LocalDevicePreference, and
defaultDeviceId) intact so failing writes are no-ops rather than runtime
exceptions.

In `@packages/client/src/devices/SpeakerManager.ts`:
- Around line 190-197: In persistSpeakerDevicePreference, the fallback synthetic
device is created with the wrong kind; open the persistSpeakerDevicePreference
function (referencing devicePersistence, listDevices, createSyntheticDevice,
writePreferences) and change the synthetic device kind from 'videoinput' to
'audiooutput' so the created device matches a speaker before calling
writePreferences with the currentDevice and storageKey.
🧹 Nitpick comments (1)
packages/react-sdk/src/hooks/usePersistedDevicePreferences.ts (1)

10-14: Consider logging the deprecation warning only once per session.

The warning currently fires on every mount and whenever key changes, which could be noisy in apps where the component using this hook remounts frequently. A common pattern is to use a module-level flag to ensure the warning logs only once.

♻️ Suggested improvement
 import { useEffect } from 'react';
 
+let warned = false;
+
 /**
  * This hook is a no-op. Device preference persistence is now handled
  * by the low-level client SDK.
  *
  * `@deprecated` use the devicePreferences API of the StreamVideoClient.
  */
 export const usePersistedDevicePreferences = (key: string = ''): void => {
   useEffect(() => {
-    console.warn(
-      `usePersistedDevicePreferences is deprecated. Please use the devicePreferences API of the StreamVideoClient instead.`,
-    );
-  }, [key]);
+    if (!warned) {
+      warned = true;
+      console.warn(
+        `usePersistedDevicePreferences is deprecated. Please use the devicePreferences API of the StreamVideoClient instead.`,
+      );
+    }
+  }, []);
 };

Comment thread packages/client/src/devices/DeviceManager.ts
Comment thread packages/client/src/devices/devicePersistence.ts
Comment thread packages/client/src/devices/SpeakerManager.ts
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
packages/client/src/devices/__tests__/mocks.ts (1)

172-215: ⚠️ Potential issue | 🟡 Minor

Fix removeEventListener to clear the audio track’s handlers.

The audio track’s removeEventListener deletes from the video track’s handler map, so audio handlers won’t be removed.

🐛 Proposed fix
-      removeEventListener(type: string) {
-        delete track.eventHandlers[type];
-      },
+      removeEventListener(type: string) {
+        delete audioTrack.eventHandlers[type];
+      },
🤖 Fix all issues with AI agents
In `@packages/client/src/devices/SpeakerManager.ts`:
- Around line 150-152: The dispose method in SpeakerManager currently uses
this.subscriptions.forEach((unsubscribe) => unsubscribe()); which returns the
callback's value and triggers a Biome error; change the implementation to avoid
returning values from forEach callbacks by either iterating with a for...of loop
over this.subscriptions and calling unsubscribe() inside the loop, or by using a
block-style callback like this.subscriptions.forEach((unsubscribe) => {
unsubscribe(); }); and then clear this.subscriptions = []; to preserve existing
behavior.
🧹 Nitpick comments (2)
packages/client/src/devices/__tests__/mocks.ts (1)

226-247: Consider moving LocalStorageMock to shared test helpers.

To keep test utilities centralized, place LocalStorageMock/createLocalStorageMock in __tests__/clientTestUtils.ts and import from there. As per coding guidelines: In test files, exclude generated code (src/gen/) from coverage and use mock helpers from tests/clientTestUtils.ts; require STREAM_API_KEY and STREAM_SECRET in .env for integration tests.

packages/client/src/devices/__tests__/devicePersistence.test.ts (1)

9-10: Use shared test helpers for the localStorage mock.

Import the localStorage mock from __tests__/clientTestUtils.ts to align with test helper conventions. As per coding guidelines: In test files, exclude generated code (src/gen/) from coverage and use mock helpers from tests/clientTestUtils.ts; require STREAM_API_KEY and STREAM_SECRET in .env for integration tests.

Comment thread packages/client/src/devices/SpeakerManager.ts
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 Fix all issues with AI agents
In `@packages/client/src/devices/DeviceManager.ts`:
- Around line 87-95: When persistence is enabled
(this.devicePersistence.enabled) we currently create a subscription using
createSubscription(combineLatest([this.state.selectedDevice$,
this.state.status$]), ...) that always calls persistPreference(selectedDevice,
status) which in turn calls mediaDeviceKind and throws for non-audio/video
managers (e.g., screen-share). Fix by gating the subscription so it is only
created for audio/video managers (check the manager type or the
selectedDevice.kind before subscribing) and additionally add a defensive early
return at the start of persistPreference(selectedDevice, status) to no-op for
non-audio/video kinds to prevent crashes; update both the subscription creation
site and persistPreference to implement these guards (also apply the same change
to the other subscription area mentioned).

In `@packages/client/src/devices/devicePersistence.ts`:
- Around line 33-62: The availability check and read of window.localStorage can
throw in SSR/RN/privacy modes; update isLocalStorageAvailable to wrap any
window/localStorage access in a try/catch and return false on error, then have
normalize rely on that safe helper (so enabled is false when access throws), and
update readPreferences to first guard with the safe isLocalStorageAvailable and
return {} if unavailable or wrap the getItem/JSON.parse in try/catch to return
{} on error; reference the functions isLocalStorageAvailable, normalize, and
readPreferences when making the changes.

Comment thread packages/client/src/devices/DeviceManager.ts
Comment thread packages/client/src/devices/devicePersistence.ts
# Conflicts:
#	packages/client/src/devices/__tests__/MicrophoneManager.test.ts
#	packages/client/src/devices/__tests__/MicrophoneManagerRN.test.ts
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (2)
packages/client/src/devices/MicrophoneManager.ts (2)

32-32: Prefer import type for the type-only import

DevicePersistenceOptions is a TypeScript type, never used as a value. The rest of the file is consistent — import type { INoiseCancellation } (line 2) and import type { BrowserPermissionState } (line 8) already follow the pattern. Using import type here keeps intent explicit and ensures the import is fully erased at compile time.

♻️ Suggested change
-import { DevicePersistenceOptions } from './devicePersistence';
+import type { DevicePersistenceOptions } from './devicePersistence';
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/client/src/devices/MicrophoneManager.ts` at line 32, Change the
value import of DevicePersistenceOptions to a type-only import: update the
import statement that currently references DevicePersistenceOptions in
MicrophoneManager.ts to use "import type" so the DevicePersistenceOptions type
is erased at compile time and remains consistent with other type-only imports
like INoiseCancellation and BrowserPermissionState.

354-357: Remove stale // enabledInCallType comment

The inline comment on line 355 is leftover development noise — it suggests the argument was once derived from something like settings.mic_default_on (the "call type default"), but true is now passed unconditionally to always restore the persisted enabled/disabled state. Replace it with a short comment that explains the actual intent.

♻️ Suggested change
-    if (shouldApplyDefaults && this.devicePersistence.enabled) {
-      // enabledInCallType
-      persistedPreferencesApplied = await this.applyPersistedPreferences(true);
-    }
+    if (shouldApplyDefaults && this.devicePersistence.enabled) {
+      // restore the full persisted state (device id + enabled/disabled)
+      persistedPreferencesApplied = await this.applyPersistedPreferences(true);
+    }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/client/src/devices/MicrophoneManager.ts` around lines 354 - 357,
Remove the stale "// enabledInCallType" comment and replace it with a concise
explanatory comment clarifying the actual intent: that when shouldApplyDefaults
is true and devicePersistence.enabled is set, we always restore the persisted
mic enabled/disabled state by calling applyPersistedPreferences(true). Update
the comment near the if block using identifiers shouldApplyDefaults,
devicePersistence.enabled, and applyPersistedPreferences to note that the
boolean argument forces restoration of the saved enabled state (not a call-type
default).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@packages/client/src/types.ts`:
- Around line 337-340: The declared CallConstructor field devicePersistence is
never read by the Call constructor (Call.ts) — the constructor always uses
streamClient.options.devicePersistence, so per-call values are ignored; either
remove devicePersistence from CallConstructor (if no per-call override is
intended) or wire it into the Call constructor by reading/destructuring
devicePersistence from the constructor args and using that value when
initializing the Call instance (instead of unconditionally using
streamClient.options.devicePersistence); update any places referencing
CallConstructor.devicePersistence accordingly and add a test that verifies
per-call devicePersistence takes effect if you implement the wiring option.

---

Nitpick comments:
In `@packages/client/src/devices/MicrophoneManager.ts`:
- Line 32: Change the value import of DevicePersistenceOptions to a type-only
import: update the import statement that currently references
DevicePersistenceOptions in MicrophoneManager.ts to use "import type" so the
DevicePersistenceOptions type is erased at compile time and remains consistent
with other type-only imports like INoiseCancellation and BrowserPermissionState.
- Around line 354-357: Remove the stale "// enabledInCallType" comment and
replace it with a concise explanatory comment clarifying the actual intent: that
when shouldApplyDefaults is true and devicePersistence.enabled is set, we always
restore the persisted mic enabled/disabled state by calling
applyPersistedPreferences(true). Update the comment near the if block using
identifiers shouldApplyDefaults, devicePersistence.enabled, and
applyPersistedPreferences to note that the boolean argument forces restoration
of the saved enabled state (not a call-type default).

Comment thread packages/client/src/types.ts Outdated
@oliverlaz
Copy link
Copy Markdown
Member Author

@coderabbitai review

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Feb 25, 2026

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Feb 25, 2026

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

@oliverlaz I'll review this PR for you.


🏁 Script executed:

#!/bin/bash

echo "=== Reviewing devicePersistence.ts implementation ==="
cat packages/client/src/devices/devicePersistence.ts

echo -e "\n=== Reviewing deprecated usePersistedDevicePreferences hook ==="
cat packages/react-sdk/src/hooks/usePersistedDevicePreferences.ts

echo -e "\n=== Reviewing DeviceManager persistence integration ==="
head -100 packages/client/src/devices/DeviceManager.ts

echo -e "\n=== Checking Call.ts device manager initialization ==="
rg -A5 -B5 "new CameraManager|new MicrophoneManager|new SpeakerManager" packages/client/src/Call.ts

echo -e "\n=== Checking StreamClientOptions for devicePersistence ==="
cat packages/client/src/coordinator/connection/types.ts | grep -A10 -B5 "devicePersistence"

Length of output: 8090


🏁 Script executed:

#!/bin/bash

echo "=== Checking DeviceManager applyPersistedPreferences implementation ==="
rg -A30 "applyPersistedPreferences" packages/client/src/devices/DeviceManager.ts

echo -e "\n=== Checking MicrophoneManager apply method with persistence ==="
rg -A40 "async apply\(" packages/client/src/devices/MicrophoneManager.ts

echo -e "\n=== Checking CameraManager apply method ==="
rg -A30 "async apply\(" packages/client/src/devices/CameraManager.ts

echo -e "\n=== Checking test coverage for device persistence ==="
cat packages/client/src/devices/__tests__/devicePersistence.test.ts | head -60

echo -e "\n=== Checking how sample app uses the new API ==="
cat sample-apps/react/react-dogfood/helpers/client.ts | grep -A10 -B10 "devicePersistence"

echo -e "\n=== Checking if there are any remaining usages of the old hook ==="
rg "usePersistedDevicePreferences" --type ts --type tsx -g '!node_modules' -g '!*.test.ts' | head -20

Length of output: 6488


#!/bin/bash

echo "=== Finding all TypeScript/TSX files with usePersistedDevicePreferences ==="
rg "usePersistedDevicePreferences" -g "*.ts" -g "*.tsx" -g '!node_modules' -g '!*.test.ts' -g '!*.test.tsx'

echo -e "\n=== Checking error handling in writePreferences ==="
rg -A5 -B5 "catch.*err" packages/client/src/devices/devicePersistence.ts

echo -e "\n=== Checking SpeakerManager persistence implementation ==="
rg -A30 "applyWeb\(\)" packages/client/src/devices/SpeakerManager.ts

echo -e "\n=== Checking if ScreenShareManager passes persistence ==="
cat packages/client/src/devices/ScreenShareManager.ts | grep -A10 "constructor"

echo -e "\n=== Checking normalize function for React Native handling ==="
rg -A10 "export const normalize" packages/client/src/devices/devicePersistence.ts

🧠 Learnings used
Learnt from: CR
Repo: GetStream/stream-video-js PR: 0
File: packages/react-sdk/CLAUDE.md:0-0
Timestamp: 2026-01-09T11:08:06.856Z
Learning: Applies to packages/react-sdk/src/hooks/usePersistedDevicePreferences.ts : Handle device selection state by checking if devices are available before use; use usePersistedDevicePreferences at app level

Learnt from: CR
Repo: GetStream/stream-video-js PR: 0
File: packages/react-sdk/CLAUDE.md:0-0
Timestamp: 2026-01-09T11:08:06.856Z
Learning: Applies to packages/react-sdk/src/**/*.{ts,tsx} : Use deprecated JSDoc with replacement guidance for deprecated APIs; avoid breaking changes and prefer additive evolution

Learnt from: CR
Repo: GetStream/stream-video-js PR: 0
File: packages/react-native-sdk/CLAUDE.md:0-0
Timestamp: 2026-01-09T11:07:38.535Z
Learning: Applies to packages/react-native-sdk/src/**/*.{ts,tsx} : Use React hooks from `stream-io/video-react-bindings` (via `useCall()` and `useCallStateHooks()`) instead of directly accessing RxJS observables from `stream-io/video-client`

Learnt from: CR
Repo: GetStream/stream-video-js PR: 0
File: sample-apps/react/react-dogfood/CLAUDE.md:0-0
Timestamp: 2026-02-03T16:27:03.014Z
Learning: Applies to sample-apps/react/react-dogfood/components/**/*.{ts,tsx} : Use SDK state hooks (useCall, useCallStateHooks, useConnectedUser, useParticipants) from stream-io/video-react-sdk instead of manual state management

Learnt from: CR
Repo: GetStream/stream-video-js PR: 0
File: packages/react-sdk/CLAUDE.md:0-0
Timestamp: 2026-01-09T11:08:06.856Z
Learning: Applies to packages/react-sdk/**/*.{ts,tsx} : Always use hooks from stream-io/video-react-bindings via useCallStateHooks() factory pattern instead of importing individual hooks directly

Learnt from: CR
Repo: GetStream/stream-video-js PR: 0
File: packages/react-sdk/CLAUDE.md:0-0
Timestamp: 2026-01-09T11:08:06.856Z
Learning: Applies to packages/react-sdk/**/{Video,Audio,ParticipantView}/**/*.{ts,tsx} : Use useLayoutEffect instead of useEffect for video element binding to call.bindVideoElement() to ensure timing before paint

Learnt from: CR
Repo: GetStream/stream-video-js PR: 0
File: packages/react-sdk/CLAUDE.md:0-0
Timestamp: 2026-01-09T11:08:06.856Z
Learning: Applies to packages/react-sdk/**/*.{ts,tsx} : Destructure useCallStateHooks() at component top level, not in dependency arrays, to avoid unnecessary re-renders

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 5

♻️ Duplicate comments (1)
packages/client/src/devices/DeviceManager.ts (1)

87-97: ⚠️ Potential issue | 🟠 Major

Guard persistence paths for supported managers only.

Persistence is subscribed/applied without enforcing audio/video + non-React-Native constraints in this class. That leaves a crash path (mediaDeviceKind throws on unsupported trackType) and may run web persistence logic in shared code paths.

🐛 Proposed fix
@@
-    if (this.devicePersistence.enabled) {
+    if (
+      this.devicePersistence.enabled &&
+      !isReactNative() &&
+      (this.trackType === TrackType.AUDIO || this.trackType === TrackType.VIDEO)
+    ) {
       this.subscriptions.push(
         createSubscription(
           combineLatest([this.state.selectedDevice$, this.state.status$]),
@@
   private persistPreference(
     selectedDevice: string | undefined,
     status: InputDeviceStatus,
   ) {
+    if (
+      this.trackType !== TrackType.AUDIO &&
+      this.trackType !== TrackType.VIDEO
+    ) {
+      return;
+    }
     const deviceKind = this.mediaDeviceKind;
@@
   protected async applyPersistedPreferences(enabledInCallType: boolean) {
+    if (
+      !this.devicePersistence.enabled ||
+      this.trackType !== TrackType.AUDIO &&
+        this.trackType !== TrackType.VIDEO
+    ) {
+      return false;
+    }
     const deviceKey: DevicePreferenceKey =
       this.trackType === TrackType.AUDIO ? 'microphone' : 'camera';

As per coding guidelines, packages/client/**/*.{ts,tsx}: "Avoid using unguarded web-only APIs in shared code that runs on React Native".

Also applies to: 576-581, 599-603

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/client/src/devices/DeviceManager.ts` around lines 87 - 97, The
persistence subscription in DeviceManager (the block that pushes
createSubscription combining this.state.selectedDevice$ and this.state.status$
and calling this.persistPreference) must only run for supported track
types/platforms; update the guard so it checks devicePersistence.enabled AND
that the current selected device’s trackType is one of the supported kinds for
this.devicePersistence (or that this.devicePersistence.supportsTrackType /
supportedTrackTypes includes selectedDevice.trackType) and also short-circuit on
React Native (e.g., skip web-only logic when running on RN). Move the guard
inside the subscription callback (or filter the combined observable) so you
never call mediaDeviceKind or persistPreference for unsupported trackTypes or on
RN; reference DeviceManager.persistPreference, devicePersistence,
this.state.selectedDevice$, this.state.status$, and createSubscription when
implementing the check.
🧹 Nitpick comments (2)
packages/client/src/devices/__tests__/MicrophoneManager.test.ts (1)

425-437: Near-duplicate tests.

The new test at lines 425–430 ("should apply defaults when mic_default_on is true and enabled is pristine") is functionally identical to the existing test at lines 432–437 ("should turn the mic on when set on dashboard"). Consider removing one.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/client/src/devices/__tests__/MicrophoneManager.test.ts` around lines
425 - 437, There are two duplicate tests both calling manager.apply({
mic_default_on: true }, true) and spying on manager.enable; remove one of them
(either "should apply defaults when mic_default_on is true and enabled is
pristine" or "should turn the mic on when set on dashboard") to avoid
redundancy, keeping the clearer test name and assertions that use
vi.spyOn(manager, 'enable') and expect(enable).toHaveBeenCalled(); ensure only a
single test covering manager.apply and manager.enable remains.
packages/client/src/devices/DeviceManager.ts (1)

512-516: Make invalid track type errors actionable.

'Invalid track type' is too generic for debugging. Include the actual value/name.

♻️ Suggested refinement
-    throw new Error('Invalid track type');
+    throw new Error(
+      `Unsupported track type for device manager: ${TrackType[this.trackType]} (${this.trackType})`,
+    );

As per coding guidelines, packages/**/*.{ts,tsx}: "Public API should throw descriptive errors or return typed error results consistent with existing patterns".

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/client/src/devices/DeviceManager.ts` around lines 512 - 516, In the
mediaDeviceKind getter of DeviceManager (the private accessor that checks
this.trackType against TrackType.AUDIO/VIDEO), replace the generic throw new
Error('Invalid track type') with a descriptive error that includes the actual
invalid value (e.g., this.trackType) and optionally its enum name, so callers
can debug what was passed; ensure the thrown message references the TrackType
value/string to make the error actionable and consistent with the project's
public API error style.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@packages/client/src/devices/__tests__/CameraManager.test.ts`:
- Around line 308-330: The test creates a CameraManager instance
(persistedManager) but never disposes it, leaking subscriptions; update the test
to clean up persistedManager by calling its dispose/close method (e.g., await
persistedManager.dispose() or persistedManager.close()) after the assertions (or
register disposal in an afterEach), ensuring any async disposal is awaited —
reference persistedManager (the CameraManager instance created around line 310)
and its apply method/use in the test so the teardown runs regardless of test
outcome.

In `@packages/client/src/devices/__tests__/DeviceManager.test.ts`:
- Around line 387-450: Tests in the persistPreference block create
TestInputMediaDeviceManager instances (constructed via new
TestInputMediaDeviceManager(...)) but never call dispose(), which can leak
subscriptions from the underlying DeviceManager; update each test that creates a
persistenceEnabledManager to call persistenceEnabledManager.dispose() at the end
(or use an afterEach that tracks and disposes created instances) so the
manager's cleanup code runs and reactive listeners/subscriptions are removed.

In `@packages/client/src/devices/__tests__/MicrophoneManager.test.ts`:
- Around line 463-480: The test creates a MicrophoneManager instance
(persistedManager) but never disposes it; after invoking
persistedManager.apply(...) ensure the manager is cleaned up by awaiting its
dispose method (e.g., call await persistedManager.dispose()) so resources are
released and spies are restored; add this disposal at the end of the test (or in
a finally block) to match the cleanup pattern used for CameraManager tests and
keep applySpy and enableSpy behavior isolated.

In `@sample-apps/react/react-dogfood/helpers/client.ts`:
- Line 2: Export the hook from the hooks barrel and update the import: add
export * from './useDeviceSelectionPreference'; to the hooks index (so the
symbol is re-exported), then change the import in client.ts to import {
isRecentDeviceSelectionEnabled } from '../hooks'; ensuring you reference the
existing isRecentDeviceSelectionEnabled function and the hooks/index.ts barrel
for the change.

In `@sample-apps/react/react-dogfood/hooks/useDeviceSelectionPreference.ts`:
- Line 5: The constant DEVICE_PREFERENCE_KEY was renamed and will silently reset
users who had the old key '@pronto/device-preferences' set to 'disabled'; add a
one-time migration in the initialization path (e.g., inside
isRecentDeviceSelectionEnabled() or the hook initializer in
useDeviceSelectionPreference.ts) that checks localStorage for the old key
'@pronto/device-preferences', if found map 'disabled' to the new boolean/flag
format (set DEVICE_PREFERENCE_KEY to false) and map any other explicit saved
value to true, write the migrated value to DEVICE_PREFERENCE_KEY, then remove
the old key so future reads use the new key only; use
localStorage.getItem/setItem/removeItem to perform the migration atomically
before returning the current preference.

---

Duplicate comments:
In `@packages/client/src/devices/DeviceManager.ts`:
- Around line 87-97: The persistence subscription in DeviceManager (the block
that pushes createSubscription combining this.state.selectedDevice$ and
this.state.status$ and calling this.persistPreference) must only run for
supported track types/platforms; update the guard so it checks
devicePersistence.enabled AND that the current selected device’s trackType is
one of the supported kinds for this.devicePersistence (or that
this.devicePersistence.supportsTrackType / supportedTrackTypes includes
selectedDevice.trackType) and also short-circuit on React Native (e.g., skip
web-only logic when running on RN). Move the guard inside the subscription
callback (or filter the combined observable) so you never call mediaDeviceKind
or persistPreference for unsupported trackTypes or on RN; reference
DeviceManager.persistPreference, devicePersistence, this.state.selectedDevice$,
this.state.status$, and createSubscription when implementing the check.

---

Nitpick comments:
In `@packages/client/src/devices/__tests__/MicrophoneManager.test.ts`:
- Around line 425-437: There are two duplicate tests both calling
manager.apply({ mic_default_on: true }, true) and spying on manager.enable;
remove one of them (either "should apply defaults when mic_default_on is true
and enabled is pristine" or "should turn the mic on when set on dashboard") to
avoid redundancy, keeping the clearer test name and assertions that use
vi.spyOn(manager, 'enable') and expect(enable).toHaveBeenCalled(); ensure only a
single test covering manager.apply and manager.enable remains.

In `@packages/client/src/devices/DeviceManager.ts`:
- Around line 512-516: In the mediaDeviceKind getter of DeviceManager (the
private accessor that checks this.trackType against TrackType.AUDIO/VIDEO),
replace the generic throw new Error('Invalid track type') with a descriptive
error that includes the actual invalid value (e.g., this.trackType) and
optionally its enum name, so callers can debug what was passed; ensure the
thrown message references the TrackType value/string to make the error
actionable and consistent with the project's public API error style.

ℹ️ Review info

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

  • Linear integration is disabled

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 326857b and 9b657ad.

📒 Files selected for processing (13)
  • packages/client/src/Call.ts
  • packages/client/src/devices/CameraManager.ts
  • packages/client/src/devices/DeviceManager.ts
  • packages/client/src/devices/MicrophoneManager.ts
  • packages/client/src/devices/ScreenShareManager.ts
  • packages/client/src/devices/__tests__/CameraManager.test.ts
  • packages/client/src/devices/__tests__/DeviceManager.test.ts
  • packages/client/src/devices/__tests__/MicrophoneManager.test.ts
  • packages/client/src/devices/__tests__/mediaStreamTestHelpers.ts
  • packages/client/src/types.ts
  • sample-apps/react/react-dogfood/components/MeetingUI.tsx
  • sample-apps/react/react-dogfood/helpers/client.ts
  • sample-apps/react/react-dogfood/hooks/useDeviceSelectionPreference.ts
🚧 Files skipped from review as they are similar to previous changes (2)
  • packages/client/src/Call.ts
  • packages/client/src/devices/ScreenShareManager.ts

Comment thread packages/client/src/devices/__tests__/CameraManager.test.ts
Comment thread packages/client/src/devices/__tests__/DeviceManager.test.ts
Comment thread packages/client/src/devices/__tests__/MicrophoneManager.test.ts
Comment thread sample-apps/react/react-dogfood/helpers/client.ts
@oliverlaz oliverlaz merged commit 7bbbd93 into main Feb 25, 2026
18 checks passed
@oliverlaz oliverlaz deleted the device-persistence branch February 25, 2026 13:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants