Repository navigation
Conversation
…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 detectedLatest commit: 9cf8465 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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 |
There was a problem hiding this comment.
Devin Review found 1 potential issue.
1 flag not posted on this PR by your GitHub settings — view it in Devin Review. (Configure)
| room.disconnect(); | ||
| }; | ||
| }, [room, sortFunc]); | ||
| }, [room]); |
There was a problem hiding this comment.
🟡 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.
Was this helpful? React with 👍 or 👎 to provide feedback.
Fixes #470
useRoomnow keeps the latestsortParticipantsin a ref (sortFuncRef), and its effect depends only onroom. Before, an inline sort function had a new identity on every render, so the effect re-ran each time and its cleanup calledroom.disconnect(). The effect also sets a newaudioTracksarray, which causes another render, so the loop did not stop.A new sort function now applies on the next participant update.
useIOSAudioManagementinsrc/audio/AudioManagerLegacy.tsuses the same pattern foronConfigureNativeAudio.src/__tests__/useRoom.test.tschecks 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 onmain.Two judgement calls:
Object.isdependency comparison. The alternative is to addreact-test-rendereror@testing-library/react-nativeas a devDependency, which changesyarn.lock. I can switch if you prefer that.package.jsonnow sets"testEnvironment": "node"for jest. Thereact-nativepreset resolvesjest-environment-node29, 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.