perf: optimize Firestore subscriptions and room state updates#856
perf: optimize Firestore subscriptions and room state updates#856Devexhhh wants to merge 1 commit into
Conversation
|
@Devexhhh is attempting to deploy a commit to the omkh4242g-1671's projects Team on Vercel. A member of the Team first needs to authorize it. |
github-actions
Bot
added
gssoc
|
Hi @Devexhhh, thanks for contributing to Debugra! 🎉 I have automatically:
Our workflows will now analyze your changes to classify:
Tip Ensure your PR description references the issue it resolves (e.g. Happy coding! 🚀 |
📝 WalkthroughWalkthroughuseRoom now memoizes derived roles, debounces cursor and room writes, skips duplicate Firestore-driven updates, batches presence updates, and deletes cursor and activeUsers records during unload and leave flows while removing access-control callbacks. ChangesuseRoom synchronization updates
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment Warning |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/hooks/useRoom.js (2)
398-406: 🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy liftDon’t rely on async Firestore writes in
beforeunload.
updateDocanddeleteDoccan be dropped when the page unloads, leaving stale active users/cursors. Use a heartbeat/TTL cleanup,pagehide/visibility handling, or server-side stale-presence pruning instead of treating unload cleanup as reliable.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/hooks/useRoom.js` around lines 398 - 406, The unload cleanup in useRoom’s useEffect relies on async Firestore writes inside handleBeforeUnload, which may be dropped during page teardown. Move presence cleanup away from beforeunload by using a heartbeat/TTL approach, pagehide or visibilitychange handling, or server-side stale presence pruning, and update the logic around roomData.activeUsers, updateDoc, and deleteDoc so disconnect cleanup does not depend on the unload event completing.
161-181: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winReset dedupe refs when room state is cleared or switched.
leaveRoomclearsroomData,activeUsers, andremoteCursors, butprevRemoteCursorsRef,prevRoomRef, andprevActiveUsersRefkeep the old snapshots. Rejoining the same unchanged room can skip the first snapshot and leave the hook in the cleared state until Firestore changes again.🐛 Proposed fix
useEffect(() => { if (!roomId) { + prevRemoteCursorsRef.current = {}; setRemoteCursors({}); return; } + prevRemoteCursorsRef.current = {}; const cursorsRef = collection(db, 'rooms', roomId, 'cursors'); @@ useEffect(() => { - if (!roomId) return; + if (!roomId) { + prevRoomRef.current = null; + prevActiveUsersRef.current = []; + return; + } + prevRoomRef.current = null; + prevActiveUsersRef.current = []; const unsub = onSnapshot(doc(db, 'rooms', roomId), (snap) => {Also applies to: 188-231
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/hooks/useRoom.js` around lines 161 - 181, The dedupe refs in useRoom are not being cleared when the room is left or switched, so leaveRoom can reset state while prevRemoteCursorsRef, prevRoomRef, and prevActiveUsersRef still hold stale snapshots. Update leaveRoom and any room-change cleanup logic to reset those refs alongside roomData, activeUsers, and remoteCursors, so rejoining the same room will not incorrectly skip the first Firestore snapshot. Make sure the fix is applied in the useEffect/onSnapshot flow that tracks room, active user, and cursor state.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/hooks/useRoom.js`:
- Around line 263-266: The update in useRoom is writing a stale copy of
activeUsers back to the room document, which can overwrite other users’ presence
changes. Refactor the activeUsers updates in the timer/unload/leave paths so
they do not rely on roomData snapshots; use a transaction that re-reads the room
immediately before updating activeUsers, or move presence to per-user documents.
Apply the same fix to the related activeUsers write sites in useRoom.
- Around line 416-421: The access-control callbacks in useRoom are currently
silent no-ops, which makes the hook expose functions that appear to succeed
while doing nothing. Update requestAccess, approveAccess, denyAccess,
revokeAccess, takeControl, and releaseControl to either perform the real
access/control actions or fail explicitly with a clear unsupported/error path,
and keep their behavior aligned with the rest of the collaboration workflow
returned by the hook. Ensure the implementation is wired through the existing
useRoom callback definitions rather than leaving the empty useCallback bodies in
place.
- Around line 428-431: The leaveRoom cleanup in useRoom is swallowing errors
with inner .catch(() => {}), so failures never reach the outer try/catch. Remove
the per-call catch handling around the deleteDoc and updateDoc operations, and
let those promises reject normally so leaveRoom can report failure through its
existing error path. Use the cleanup block in useRoom that updates room cursors
and activeUsers to make this change.
- Around line 130-145: The cursor dedupe in useRoom should not rely only on
line/col, because the same position can belong to a different room or user and
then the write is incorrectly skipped. Update the prevCursorRef guard inside the
useEffect in useRoom to include room/user identity (for example roomId and user
id/name) along with cursorPos, and reset or re-evaluate the cached cursor when
those identities change so a fresh cursor document is written after
room/user/editor-state transitions.
- Around line 204-235: The room snapshot listener in useRoom is being recreated
on every local edit because the onSnapshot effect depends on code, language, and
stdinValue. Refactor the listener so it reads the latest values from refs (or
another stable source) instead of effect dependencies, keeping the useEffect
that sets up onSnapshot stable. Update the comparisons in the snapshot callback
to use those refs, and keep roomId, user, and the setter functions as the only
dependencies needed for subscription setup.
---
Outside diff comments:
In `@src/hooks/useRoom.js`:
- Around line 398-406: The unload cleanup in useRoom’s useEffect relies on async
Firestore writes inside handleBeforeUnload, which may be dropped during page
teardown. Move presence cleanup away from beforeunload by using a heartbeat/TTL
approach, pagehide or visibilitychange handling, or server-side stale presence
pruning, and update the logic around roomData.activeUsers, updateDoc, and
deleteDoc so disconnect cleanup does not depend on the unload event completing.
- Around line 161-181: The dedupe refs in useRoom are not being cleared when the
room is left or switched, so leaveRoom can reset state while
prevRemoteCursorsRef, prevRoomRef, and prevActiveUsersRef still hold stale
snapshots. Update leaveRoom and any room-change cleanup logic to reset those
refs alongside roomData, activeUsers, and remoteCursors, so rejoining the same
room will not incorrectly skip the first Firestore snapshot. Make sure the fix
is applied in the useEffect/onSnapshot flow that tracks room, active user, and
cursor state.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: c65a53df-e03c-4582-8375-f59c75d5cc1f
📒 Files selected for processing (1)
src/hooks/useRoom.js
| const prevCursorRef = useRef({ line: null, col: null }); | ||
|
|
||
| useEffect(() => { | ||
| if (!roomId || !user || !isEditor || !cursorPos) return; | ||
|
|
||
| // 5. Skip duplicate cursor updates | ||
| if ( | ||
| prevCursorRef.current.line === cursorPos.line && | ||
| prevCursorRef.current.col === cursorPos.col | ||
| ) { | ||
| return; | ||
| } | ||
|
|
||
| // 4. Batch cursor writes | ||
| const timer = setTimeout(() => { | ||
| prevCursorRef.current = { line: cursorPos.line, col: cursorPos.col }; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Key cursor dedupe by room and user, not only position.
Line 136 skips writes when the coordinates match the previous write. After leaving/joining another room, switching users, or regaining editor rights at the same cursor position, the new cursor doc may never be written.
🐛 Proposed fix
- const prevCursorRef = useRef({ line: null, col: null });
+ const prevCursorRef = useRef({ roomId: null, uid: null, line: null, col: null });
@@
if (
+ prevCursorRef.current.roomId === roomId &&
+ prevCursorRef.current.uid === user.uid &&
prevCursorRef.current.line === cursorPos.line &&
prevCursorRef.current.col === cursorPos.col
) {
return;
}
@@
- prevCursorRef.current = { line: cursorPos.line, col: cursorPos.col };
+ prevCursorRef.current = {
+ roomId,
+ uid: user.uid,
+ line: cursorPos.line,
+ col: cursorPos.col,
+ };📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const prevCursorRef = useRef({ line: null, col: null }); | |
| useEffect(() => { | |
| if (!roomId || !user || !isEditor || !cursorPos) return; | |
| // 5. Skip duplicate cursor updates | |
| if ( | |
| prevCursorRef.current.line === cursorPos.line && | |
| prevCursorRef.current.col === cursorPos.col | |
| ) { | |
| return; | |
| } | |
| // 4. Batch cursor writes | |
| const timer = setTimeout(() => { | |
| prevCursorRef.current = { line: cursorPos.line, col: cursorPos.col }; | |
| const prevCursorRef = useRef({ roomId: null, uid: null, line: null, col: null }); | |
| useEffect(() => { | |
| if (!roomId || !user || !isEditor || !cursorPos) return; | |
| // 5. Skip duplicate cursor updates | |
| if ( | |
| prevCursorRef.current.roomId === roomId && | |
| prevCursorRef.current.uid === user.uid && | |
| prevCursorRef.current.line === cursorPos.line && | |
| prevCursorRef.current.col === cursorPos.col | |
| ) { | |
| return; | |
| } | |
| // 4. Batch cursor writes | |
| const timer = setTimeout(() => { | |
| prevCursorRef.current = { | |
| roomId, | |
| uid: user.uid, | |
| line: cursorPos.line, | |
| col: cursorPos.col, | |
| }; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/hooks/useRoom.js` around lines 130 - 145, The cursor dedupe in useRoom
should not rely only on line/col, because the same position can belong to a
different room or user and then the write is incorrectly skipped. Update the
prevCursorRef guard inside the useEffect in useRoom to include room/user
identity (for example roomId and user id/name) along with cursorPos, and reset
or re-evaluate the cached cursor when those identities change so a fresh cursor
document is written after room/user/editor-state transitions.
| if ( | ||
| data.code !== undefined && | ||
| data.code !== code && | ||
| data._lastEditor !== user?.uid | ||
| ) { | ||
| setCode(data.code); | ||
| } | ||
|
|
||
| if ( | ||
| data.language !== undefined && | ||
| data.language !== language && | ||
| data._lastEditor !== user?.uid | ||
| ) { | ||
| setLanguage(data.language); | ||
| } | ||
|
|
||
| if ( | ||
| data.stdin !== undefined && | ||
| data.stdin !== stdinValue && | ||
| data._lastEditor !== user?.uid | ||
| ) { | ||
| setStdinValue(data.stdin); | ||
| } | ||
|
|
||
| const incomingUsers = data.activeUsers || []; | ||
| if (JSON.stringify(prevActiveUsersRef.current) !== JSON.stringify(incomingUsers)) { | ||
| prevActiveUsersRef.current = incomingUsers; | ||
| setActiveUsers(incomingUsers); | ||
| } | ||
| }); | ||
| return unsub; | ||
| }, [roomId, user, setCode, setLanguage, setStdinValue]); | ||
| }, [roomId, user, code, language, stdinValue, setCode, setLanguage, setStdinValue]); |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🟠 Major | ⚡ Quick win
Keep the room snapshot listener stable during edits.
The listener reads code, language, and stdinValue, so every local edit tears down and recreates onSnapshot. That reintroduces listener churn and extra initial snapshots on the hot editing path this PR is trying to optimize.
⚡ Proposed fix
+ const latestLocalStateRef = useRef({ code, language, stdinValue });
+
+ useEffect(() => {
+ latestLocalStateRef.current = { code, language, stdinValue };
+ }, [code, language, stdinValue]);
+
@@
if (
data.code !== undefined &&
- data.code !== code &&
+ data.code !== latestLocalStateRef.current.code &&
data._lastEditor !== user?.uid
) {
setCode(data.code);
}
@@
data.language !== undefined &&
- data.language !== language &&
+ data.language !== latestLocalStateRef.current.language &&
data._lastEditor !== user?.uid
) {
setLanguage(data.language);
}
@@
data.stdin !== undefined &&
- data.stdin !== stdinValue &&
+ data.stdin !== latestLocalStateRef.current.stdinValue &&
data._lastEditor !== user?.uid
) {
setStdinValue(data.stdin);
}
@@
- }, [roomId, user, code, language, stdinValue, setCode, setLanguage, setStdinValue]);
+ }, [roomId, user?.uid, setCode, setLanguage, setStdinValue]);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if ( | |
| data.code !== undefined && | |
| data.code !== code && | |
| data._lastEditor !== user?.uid | |
| ) { | |
| setCode(data.code); | |
| } | |
| if ( | |
| data.language !== undefined && | |
| data.language !== language && | |
| data._lastEditor !== user?.uid | |
| ) { | |
| setLanguage(data.language); | |
| } | |
| if ( | |
| data.stdin !== undefined && | |
| data.stdin !== stdinValue && | |
| data._lastEditor !== user?.uid | |
| ) { | |
| setStdinValue(data.stdin); | |
| } | |
| const incomingUsers = data.activeUsers || []; | |
| if (JSON.stringify(prevActiveUsersRef.current) !== JSON.stringify(incomingUsers)) { | |
| prevActiveUsersRef.current = incomingUsers; | |
| setActiveUsers(incomingUsers); | |
| } | |
| }); | |
| return unsub; | |
| }, [roomId, user, setCode, setLanguage, setStdinValue]); | |
| }, [roomId, user, code, language, stdinValue, setCode, setLanguage, setStdinValue]); | |
| const latestLocalStateRef = useRef({ code, language, stdinValue }); | |
| useEffect(() => { | |
| latestLocalStateRef.current = { code, language, stdinValue }; | |
| }, [code, language, stdinValue]); | |
| if ( | |
| data.code !== undefined && | |
| data.code !== latestLocalStateRef.current.code && | |
| data._lastEditor !== user?.uid | |
| ) { | |
| setCode(data.code); | |
| } | |
| if ( | |
| data.language !== undefined && | |
| data.language !== latestLocalStateRef.current.language && | |
| data._lastEditor !== user?.uid | |
| ) { | |
| setLanguage(data.language); | |
| } | |
| if ( | |
| data.stdin !== undefined && | |
| data.stdin !== latestLocalStateRef.current.stdinValue && | |
| data._lastEditor !== user?.uid | |
| ) { | |
| setStdinValue(data.stdin); | |
| } | |
| const incomingUsers = data.activeUsers || []; | |
| if (JSON.stringify(prevActiveUsersRef.current) !== JSON.stringify(incomingUsers)) { | |
| prevActiveUsersRef.current = incomingUsers; | |
| setActiveUsers(incomingUsers); | |
| } | |
| }); | |
| return unsub; | |
| }, [roomId, user?.uid, setCode, setLanguage, setStdinValue]); |
🧰 Tools
🪛 ast-grep (0.44.0)
[warning] 230-230: Avoid using the initial state variable in setState
Context: setActiveUsers(incomingUsers)
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.
(setstate-same-var)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/hooks/useRoom.js` around lines 204 - 235, The room snapshot listener in
useRoom is being recreated on every local edit because the onSnapshot effect
depends on code, language, and stdinValue. Refactor the listener so it reads the
latest values from refs (or another stable source) instead of effect
dependencies, keeping the useEffect that sets up onSnapshot stable. Update the
comparisons in the snapshot callback to use those refs, and keep roomId, user,
and the setter functions as the only dependencies needed for subscription setup.
| const timer = setTimeout(() => { | ||
| const newUsers = [...currentUsers]; | ||
| newUsers[myIndex] = { ...newUsers[myIndex], activeFile: language }; | ||
| updateDoc(doc(db, 'rooms', roomId), { activeUsers: newUsers }).catch(() => { }); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Avoid overwriting activeUsers from stale snapshots.
These paths write the entire activeUsers array after reading it from roomData. With batching/unload/leave, another user can join, leave, or change activeFile before this write lands, and the stale array will clobber their update.
Prefer a per-user presence document or a transaction that re-reads the room immediately before updating activeUsers.
Also applies to: 404-406, 428-431
🧰 Tools
🪛 ast-grep (0.44.0)
[warning] 262-266: Avoid using the initial state variable in setState
Context: setTimeout(() => {
const newUsers = [...currentUsers];
newUsers[myIndex] = { ...newUsers[myIndex], activeFile: language };
updateDoc(doc(db, 'rooms', roomId), { activeUsers: newUsers }).catch(() => { });
}, PRESENCE_BATCH_DELAY)
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.
(setstate-same-var)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/hooks/useRoom.js` around lines 263 - 266, The update in useRoom is
writing a stale copy of activeUsers back to the room document, which can
overwrite other users’ presence changes. Refactor the activeUsers updates in the
timer/unload/leave paths so they do not rely on roomData snapshots; use a
transaction that re-reads the room immediately before updating activeUsers, or
move presence to per-user documents. Apply the same fix to the related
activeUsers write sites in useRoom.
| const requestAccess = useCallback(() => { }, []); | ||
| const approveAccess = useCallback(() => { }, []); | ||
| const denyAccess = useCallback(() => { }, []); | ||
| const revokeAccess = useCallback(() => { }, []); | ||
| const takeControl = useCallback(() => { }, []); | ||
| const releaseControl = useCallback(() => { }, []); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
Don’t leave public access-control callbacks as silent no-ops.
The hook still returns requestAccess, approveAccess, denyAccess, revokeAccess, takeControl, and releaseControl, but callers now get successful no-op callbacks. That silently breaks existing access/control UI flows while the PR objective says collaboration workflow is preserved.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/hooks/useRoom.js` around lines 416 - 421, The access-control callbacks in
useRoom are currently silent no-ops, which makes the hook expose functions that
appear to succeed while doing nothing. Update requestAccess, approveAccess,
denyAccess, revokeAccess, takeControl, and releaseControl to either perform the
real access/control actions or fail explicitly with a clear unsupported/error
path, and keep their behavior aligned with the rest of the collaboration
workflow returned by the hook. Ensure the implementation is wired through the
existing useRoom callback definitions rather than leaving the empty useCallback
bodies in place.
| await deleteDoc(doc(db, 'rooms', roomId, 'cursors', user.uid)).catch(() => { }); | ||
| if (roomData) { | ||
| const newUsers = (roomData.activeUsers || []).filter((u) => u.uid !== user.uid); | ||
| await updateDoc(doc(db, 'rooms', roomId), { activeUsers: newUsers }).catch(() => {}); | ||
| await updateDoc(doc(db, 'rooms', roomId), { activeUsers: newUsers }).catch(() => { }); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Let leave cleanup failures reach the outer catch.
The inner .catch(() => {}) calls make the surrounding try/catch ineffective, so leaveRoom shows success even when cursor deletion or presence cleanup fails.
🐛 Proposed fix
- await deleteDoc(doc(db, 'rooms', roomId, 'cursors', user.uid)).catch(() => { });
+ await deleteDoc(doc(db, 'rooms', roomId, 'cursors', user.uid));
if (roomData) {
const newUsers = (roomData.activeUsers || []).filter((u) => u.uid !== user.uid);
- await updateDoc(doc(db, 'rooms', roomId), { activeUsers: newUsers }).catch(() => { });
+ await updateDoc(doc(db, 'rooms', roomId), { activeUsers: newUsers });
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| await deleteDoc(doc(db, 'rooms', roomId, 'cursors', user.uid)).catch(() => { }); | |
| if (roomData) { | |
| const newUsers = (roomData.activeUsers || []).filter((u) => u.uid !== user.uid); | |
| await updateDoc(doc(db, 'rooms', roomId), { activeUsers: newUsers }).catch(() => {}); | |
| await updateDoc(doc(db, 'rooms', roomId), { activeUsers: newUsers }).catch(() => { }); | |
| await deleteDoc(doc(db, 'rooms', roomId, 'cursors', user.uid)); | |
| if (roomData) { | |
| const newUsers = (roomData.activeUsers || []).filter((u) => u.uid !== user.uid); | |
| await updateDoc(doc(db, 'rooms', roomId), { activeUsers: newUsers }); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/hooks/useRoom.js` around lines 428 - 431, The leaveRoom cleanup in
useRoom is swallowing errors with inner .catch(() => {}), so failures never
reach the outer try/catch. Remove the per-call catch handling around the
deleteDoc and updateDoc operations, and let those promises reject normally so
leaveRoom can report failure through its existing error path. Use the cleanup
block in useRoom that updates room cursors and activeUsers to make this change.
1 participant
✦ Description
This PR improves the performance and scalability of the
useRoomhook by optimizing Firestore subscriptions, reducing redundant state updates, and minimizing unnecessary synchronization during collaborative editing.As the number of collaborators increases, the existing implementation may trigger excessive Firestore reads, repeated React state updates, and unnecessary component re-renders. These changes improve the efficiency of the current synchronization pipeline while preserving the existing collaboration behavior.
🚀 Changes Made
🔄 Firestore Subscription Optimizations
Reduced redundant Firestore snapshot processing.
Prevented unnecessary state updates when incoming room data has not changed.
Reduced duplicate updates triggered by repeated Firestore snapshots.
Improved subscription efficiency during active collaboration.
⚡ Presence & Cursor Optimizations
Batched presence and cursor synchronization to reduce Firestore write frequency.
Prevented duplicate cursor updates when cursor position remains unchanged.
Reduced unnecessary presence updates during continuous editing.
🧠 State Management Improvements
Memoized derived room permission values (
isHost,isEditor,isReadOnly, etc.) to avoid unnecessary recomputation.Reduced unnecessary React re-renders by avoiding duplicate state updates.
Optimized synchronization flow by only updating local state when values actually change.
🛠 Code Quality Improvements
Extracted synchronization timing values into reusable constants.
Improved cleanup of listeners and timers to avoid unnecessary work.
Refactored synchronization logic for improved readability and maintainability.
📈 Benefits
Reduced Firestore reads and writes.
Reduced unnecessary React renders.
Improved responsiveness in collaborative rooms.
Lower network overhead during active collaboration.
Better scalability for rooms with multiple active participants.
Improved maintainability of the synchronization logic.
Scope
This PR focuses on optimizing the existing Firestore subscription model.
It does not introduce architectural changes such as listener multiplexing, WebSocket synchronization, or alternative real-time backends. Instead, it improves the efficiency of the current Firestore-based collaboration system while maintaining backward compatibility.
Fixes #817
⟡ Type of Change
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)
Documentation update (non-breaking change to docs)
Code styling/formatting (prettier, eslint, spacing)
✦ Checklist
My code follows the style guidelines of this project.
I have performed a self-review of my code.
I have commented my code, particularly in hard-to-understand areas.
My changes generate no new warnings or console errors.
I have verified that my changes work correctly on both desktop and mobile viewports.
(If applicable) I have run
npm run lintandnpm run formatlocally before pushing.Additional Notes
This PR addresses the performance concerns related to multiple Firestore subscriptions by reducing redundant snapshot processing, batching synchronization updates, and minimizing unnecessary React state changes.
The implemented optimizations improve the scalability of the
useRoomhook and reduce UI lag in larger collaborative sessions without altering the existing collaboration workflow or introducing breaking changes.Summary by CodeRabbit