Skip to content

fix: keep useRoom from disconnecting when sortParticipants changes identity - #472

Open
cpruijsen wants to merge 1 commit into
livekit:mainfrom
cpruijsen:fix/issue-470-93807c63
Open

cpruijsen wants to merge 1 commit into
livekit:mainfrom
cpruijsen:fix/issue-470-93807c63

Conversation

@cpruijsen

Copy link
Copy Markdown

Fixes #470

useRoom now keeps the latest sortParticipants in a ref (sortFuncRef), and its effect depends only on room. Before, an inline sort function had a new identity on every render, so the effect re-ran each time and its cleanup called room.disconnect(). The effect also sets a new audioTracks array, which causes another render, so the loop did not stop.

A new sort function now applies on the next participant update. useIOSAudioManagement in src/audio/AudioManagerLegacy.ts uses the same pattern for onConfigureNativeAudio.

src/__tests__/useRoom.test.ts checks that a re-render with a new inline sort function does not disconnect, that unmount disconnects once, and that the latest sort function is used. The first and third cases fail on main.

Two judgement calls:

  • The repo has no test renderer, so the test mocks the React hooks and replays the Object.is dependency comparison. The alternative is to add react-test-renderer or @testing-library/react-native as a devDependency, which changes yarn.lock. I can switch if you prefer that.
  • package.json now sets "testEnvironment": "node" for jest. The react-native preset resolves jest-environment-node 29, which fails under jest 30 before any test runs (clearMocksOnScope is not a function). CI passes today only because there are no tests (--passWithNoTests).

Changeset included.

…entity

An inline sortParticipants passed to useRoom has a new identity on every
render. The effect listed the sort function as a dependency and its
cleanup calls room.disconnect(), so every render disconnected the room
and the resulting state update re-rendered, looping forever.

Keep the latest sort function in a ref so the effect depends only on the
room. Add a jest test covering the regression, and set testEnvironment
to node so the suite runs under jest 30 (the react-native preset's
environment resolves jest-environment-node 29, which is incompatible).
@changeset-bot

changeset-bot Bot commented Oct 8, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 9cf8465

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@livekit/react-native Patch

Not sure what this means? Click here to learn what changesets are.

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

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Devin Review found 1 potential issue.

1 flag not posted on this PR by your GitHub settings — view it in Devin Review. (Configure)

Devin Review

Comment thread src/useRoom.ts
room.disconnect();
};
}, [room, sortFunc]);
}, [room]);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 Participant order stays stale after sorter changes

When sortParticipants changes without a room event, useRoom leaves the displayed participants in their old order. onParticipantsChanged only sorts again on room events, so the new order can remain invisible indefinitely.

Learn more

The hook stores the ordered participant list in React state. Updating the sorter ref changes which callback the event listener uses later, but does not rebuild that list. Previously a new callback identity restarted the effect and immediately sorted the current room participants. Avoid restarting that effect because its cleanup disconnects the room; instead trigger a participant-only state update after a committed sorter change, without treating every new inline callback as a reason to synchronously rebuild state in a loop.

Example: A room has Alice and Bob, with Alice first. A settings change supplies a sorter that puts Bob first, but no participant or track event follows. The hook continues returning Alice first instead of Bob first.

Recommended fix: Separate the participant refresh from the room subscription lifecycle. Refresh when the configured sorting behavior changes while preventing inline callbacks that change identity every render from causing a render loop; validate both ordering changes and the no-disconnect case in tests.

Devin Review


Was this helpful? React with 👍 or 👎 to provide feedback.

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.

useRoom disconnects the room on every render with an inline sortParticipants

1 participant