chore: Replace useSafeRefCallback from ui-client with fuselage-hooks' version
#38201
Conversation
|
Looks like this PR is ready to merge! 🎉 |
|
WalkthroughThis PR migrates consumers of Changes
Sequence Diagram(s)(omitted) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro Disabled knowledge base sources:
📒 Files selected for processing (14)
💤 Files with no reviewable changes (4)
🚧 Files skipped from review as they are similar to previous changes (3)
🧰 Additional context used📓 Path-based instructions (1)**/*.{ts,tsx,js}📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
Files:
🧠 Learnings (4)📚 Learning: 2025-09-25T09:59:26.461ZApplied to files:
📚 Learning: 2025-10-28T16:53:42.761ZApplied to files:
📚 Learning: 2025-11-04T16:49:19.107ZApplied to files:
📚 Learning: 2025-11-19T18:20:07.720ZApplied to files:
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
🔇 Additional comments (7)
✏️ Tip: You can disable this entire section by setting 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@apps/meteor/client/views/room/MessageList/hooks/useJumpToMessage.ts`:
- Around line 51-54: In useJumpToMessage's callback (the arrow fn with parameter
node: HTMLElement) the guard incorrectly checks an undefined symbol "scroll";
remove that check and only guard on listRef/existing values that are in scope
(e.g., if (!listRef || !node) return;), or replace it with the correct in-scope
ref if there was meant to be a scrollRef name. Update the guard to reference
only declared variables (listRef, node, or an actual scrollRef) and remove the
stray "scroll" check so the function's behavior matches the refactor.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (14)
apps/meteor/client/components/message/content/attachments/file/hooks/useReloadOnError.tsxapps/meteor/client/views/room/MessageList/hooks/useJumpToMessage.tsapps/meteor/client/views/room/body/hooks/useGetMore.tsapps/meteor/client/views/room/body/hooks/useListIsAtBottom.tsapps/meteor/client/views/room/body/hooks/useRestoreScrollPosition.tsapps/meteor/client/views/room/composer/RoomComposer/hooks/useAutoGrow.tsapps/meteor/client/views/room/composer/messageBox/MessageBox.tsxapps/meteor/client/views/room/hooks/useIsVisible.tspackages/ui-client/src/hooks/index.tspackages/ui-client/src/hooks/useSafeRefCallback/index.tspackages/ui-client/src/hooks/useSafeRefCallback/useSafeRefCallback.spec.tsxpackages/ui-client/src/hooks/useSafeRefCallback/useSafeRefCallback.tspackages/ui-voip/src/context/useMediaStream.tspackages/ui-voip/src/hooks/VoipPopupDraggable/DraggableCore.ts
💤 Files with no reviewable changes (4)
- packages/ui-client/src/hooks/useSafeRefCallback/index.ts
- packages/ui-client/src/hooks/useSafeRefCallback/useSafeRefCallback.ts
- packages/ui-client/src/hooks/index.ts
- packages/ui-client/src/hooks/useSafeRefCallback/useSafeRefCallback.spec.tsx
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
**/*.{ts,tsx,js}: Write concise, technical TypeScript/JavaScript with accurate typing in Playwright tests
Avoid code comments in the implementation
Files:
packages/ui-voip/src/context/useMediaStream.tsapps/meteor/client/views/room/hooks/useIsVisible.tsapps/meteor/client/components/message/content/attachments/file/hooks/useReloadOnError.tsxapps/meteor/client/views/room/composer/messageBox/MessageBox.tsxpackages/ui-voip/src/hooks/VoipPopupDraggable/DraggableCore.tsapps/meteor/client/views/room/composer/RoomComposer/hooks/useAutoGrow.tsapps/meteor/client/views/room/body/hooks/useListIsAtBottom.tsapps/meteor/client/views/room/MessageList/hooks/useJumpToMessage.tsapps/meteor/client/views/room/body/hooks/useGetMore.tsapps/meteor/client/views/room/body/hooks/useRestoreScrollPosition.ts
🧠 Learnings (3)
📚 Learning: 2025-10-28T16:53:42.761Z
Learnt from: ricardogarim
Repo: RocketChat/Rocket.Chat PR: 37205
File: ee/packages/federation-matrix/src/FederationMatrix.ts:296-301
Timestamp: 2025-10-28T16:53:42.761Z
Learning: In the Rocket.Chat federation-matrix integration (ee/packages/federation-matrix/), the createRoom method from rocket.chat/federation-sdk will support a 4-argument signature (userId, roomName, visibility, displayName) in newer versions. Code using this 4-argument call is forward-compatible with planned library updates and should not be flagged as an error.
Applied to files:
apps/meteor/client/views/room/hooks/useIsVisible.tsapps/meteor/client/views/room/composer/messageBox/MessageBox.tsx
📚 Learning: 2025-11-04T16:49:19.107Z
Learnt from: ricardogarim
Repo: RocketChat/Rocket.Chat PR: 37377
File: apps/meteor/ee/server/hooks/federation/index.ts:86-88
Timestamp: 2025-11-04T16:49:19.107Z
Learning: In Rocket.Chat's federation system (apps/meteor/ee/server/hooks/federation/), permission checks follow two distinct patterns: (1) User-initiated federation actions (creating rooms, adding users to federated rooms, joining from invites) should throw MeteorError to inform users they lack 'access-federation' permission. (2) Remote server-initiated federation events should silently skip/ignore when users lack permission. The beforeAddUserToRoom hook only executes for local user-initiated actions, so throwing an error there is correct. Remote federation events are handled separately by the federation Matrix package with silent skipping logic.
Applied to files:
apps/meteor/client/views/room/composer/messageBox/MessageBox.tsx
📚 Learning: 2025-11-19T18:20:07.720Z
Learnt from: gabriellsh
Repo: RocketChat/Rocket.Chat PR: 37419
File: packages/i18n/src/locales/en.i18n.json:918-921
Timestamp: 2025-11-19T18:20:07.720Z
Learning: Repo: RocketChat/Rocket.Chat — i18n/formatting
Learning: This repository uses a custom message formatting parser in UI blocks/messages; do not assume standard Markdown rules. For keys like Call_ended_bold, Call_not_answered_bold, Call_failed_bold, and Call_transferred_bold in packages/i18n/src/locales/en.i18n.json, retain the existing single-asterisk emphasis unless maintainers request otherwise.
Applied to files:
apps/meteor/client/views/room/composer/messageBox/MessageBox.tsx
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: 📦 Build Packages
- GitHub Check: cubic · AI code reviewer
- GitHub Check: CodeQL-Build
- GitHub Check: CodeQL-Build
🔇 Additional comments (17)
apps/meteor/client/views/room/hooks/useIsVisible.ts (1)
1-1: LGTM! Import migration and callback signature are correct.The import consolidation from
@rocket.chat/fuselage-hooksis valid, and the removal of the| nulltype from the callback parameter aligns with howuseSafeRefCallbackis used throughout the codebase. The cleanup function pattern (lines 18-20) correctly returns the observer disconnect handler.apps/meteor/client/views/room/MessageList/hooks/useJumpToMessage.ts (1)
2-2: LGTM!Import consolidation correctly aligns with the PR objective of migrating
useSafeRefCallbackto@rocket.chat/fuselage-hooks.apps/meteor/client/views/room/body/hooks/useGetMore.ts (2)
1-1: LGTM!Import migration from
@rocket.chat/ui-clientto@rocket.chat/fuselage-hooksis correct.
20-94: LGTM - Safe removal of null guard.The assumption that
useSafeRefCallbackonly invokes the callback with non-null elements is correct. This pattern is consistently used across 10+ files in the codebase (useIsVisible.ts, MessageBox.tsx, useAutoGrow.ts, useRestoreScrollPosition.ts, useJumpToMessage.ts, useListIsAtBottom.ts, useReloadOnError.tsx, and others), all with non-null element types and no null guards in the callback body.apps/meteor/client/views/room/body/hooks/useListIsAtBottom.ts (2)
1-1: LGTM!Clean import consolidation from
@rocket.chat/fuselage-hooks.
35-70: Implementation signature is correct.The callback signature
(node: HTMLElement)is appropriate foruseSafeRefCallback. The codebase consistently uses this pattern across multiple files without null checks (e.g.,useRestoreScrollPosition.ts), confirming the hook guarantees non-null node invocation. The cleanup function is properly returned.packages/ui-voip/src/context/useMediaStream.ts (2)
1-1: Import migration is correct.The import source change from
@rocket.chat/ui-clientto@rocket.chat/fuselage-hooksis confirmed. TheuseSafeRefCallbackAPI is consistently used across the codebase from the same source, and no conflicts or incompatibilities are present.
30-52: No issues found.The code is correct.
useSafeRefCallbackfilters null values before invoking the inner callback, making the type narrowing fromHTMLAudioElement | nulltoHTMLAudioElementsafe and thenode.srcObjectaccess valid.apps/meteor/client/views/room/body/hooks/useRestoreScrollPosition.ts (2)
1-1: API compatibility ofuseSafeRefCallbackfrom fuselage-hooks is verified.The hook from
@rocket.chat/fuselage-hooksis already consistently used across the codebase (useMediaStream.ts, DraggableCore.ts, useJumpToMessage.ts, useListIsAtBottom.ts, useGetMore.ts, useIsVisible.ts, MessageBox.tsx, useAutoGrow.ts, useReloadOnError.tsx) with the same callback signature pattern: non-nullable element types (HTMLElement, HTMLAudioElement, HTMLTextAreaElement) with direct property access and no null checks. The import change maintains the same API contract and is safe.
10-34: Unable to definitively verify the null-safety assumption without access to the@rocket.chat/fuselage-hooksimplementation source. The codebase pattern and type signatures suggest thatuseSafeRefCallbacksafely handles null before invoking the callback (only calling it when the element is mounted), which would make the non-nullable callback signature(node: HTMLElement)correct. However, this requires verification against the external package's implementation to confirm.packages/ui-voip/src/hooks/VoipPopupDraggable/DraggableCore.ts (1)
3-3: Verify thatuseSafeRefCallbackfrom@rocket.chat/fuselage-hooksguards against null before invoking the callback.React ref callbacks receive
nullon unmount. The three wrapped callbacks (lines 534, 543, 565) lack internal null-checks but call methods likegetBoundingClientRect()that would fail on null. Confirm the fuselage-hooks implementation does not pass null to the provided callback, or add null-guards within each callback.apps/meteor/client/views/room/composer/messageBox/MessageBox.tsx (2)
3-3: LGTM!Clean import consolidation—grouping
useSafeRefCallbackwith other hooks from@rocket.chat/fuselage-hooks.
357-369: The callback's null-handling is safe.useSafeRefCallbackfrom@rocket.chat/fuselage-hooksguarantees that the callback is only invoked when the node is non-null and runs cleanup when the ref becomes null or the component unmounts. The code correctly relies on this contract—no null guard is needed, and no runtime errors will occur on unmount.apps/meteor/client/components/message/content/attachments/file/hooks/useReloadOnError.tsx (2)
1-1: LGTM on import consolidation.Clean consolidation of both hooks from the same package.
135-148: VerifyuseSafeRefCallbackcontract in library documentation.The callback assumes
nodeis always non-null and directly invokes methods without null checks. Confirm thatuseSafeRefCallbackfrom@rocket.chat/fuselage-hooks(v0.38.1) guarantees the callback is only invoked when the element is mounted, never withnull. If this contract is not documented or guaranteed by the library, add a null guard in the callback before accessing the node.apps/meteor/client/views/room/composer/RoomComposer/hooks/useAutoGrow.ts (2)
1-1: Import migration looks correct.The import source change from
@rocket.chat/ui-clientto@rocket.chat/fuselage-hooksaligns with the PR objective.
19-47: Verify the null-handling contract ofuseSafeRefCallback.The callback accesses
nodedirectly without null checks (e.g.,shouldScrollToBottom(node),node.style.height,node.addEventListener). React'sRefCallback<T>type signature is(instance: T | null) => void, meaning the callback can be invoked withnullwhen the ref is removed or unmount occurs. This code assumesuseSafeRefCallbackprevents null calls; without confirming this behavior in the fuselage-hooks implementation, the code may be unsafe. Either add a null guard (if (!node) return;) or document the assumption thatuseSafeRefCallbackguarantees non-null invocation.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #38201 +/- ##
===========================================
+ Coverage 70.65% 70.67% +0.02%
===========================================
Files 3136 3135 -1
Lines 108627 108592 -35
Branches 19574 19549 -25
===========================================
+ Hits 76745 76746 +1
+ Misses 29872 29834 -38
- Partials 2010 2012 +2
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
📦 Docker Image Size Report➡️ Changes
📊 Historical Trend---
config:
theme: "dark"
xyChart:
width: 900
height: 400
---
xychart
title "Image Size Evolution by Service (Last 30 Days + This PR)"
x-axis ["11/18 22:53", "11/19 23:02", "11/21 16:49", "11/24 17:34", "11/27 22:32", "11/28 19:05", "12/01 23:01", "12/02 21:57", "12/03 21:00", "12/04 18:17", "12/05 21:56", "12/08 20:15", "12/09 22:17", "12/10 23:26", "12/11 21:56", "12/12 22:45", "12/13 01:34", "12/15 22:31", "12/16 22:18", "12/17 21:04", "12/18 23:12", "12/19 23:27", "12/20 21:03", "12/22 18:54", "12/23 16:16", "12/24 19:38", "12/25 17:51", "12/26 13:18", "12/29 19:01", "12/30 20:52", "01/17 15:14 (PR)"]
y-axis "Size (GB)" 0 --> 0.5
line "account-service" [0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.00]
line "authorization-service" [0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.00]
line "ddp-streamer-service" [0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.00]
line "omnichannel-transcript-service" [0.14, 0.14, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.00]
line "presence-service" [0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.00]
line "queue-worker-service" [0.14, 0.14, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.13, 0.00]
line "rocketchat" [0.35, 0.35, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.34, 0.00]
Statistics (last 30 days):
ℹ️ About this reportThis report compares Docker image sizes from this build against the
Updated: Sat, 17 Jan 2026 15:14:22 GMT |
juliajforesti
added
the
stat: QA assured
juliajforesti
force-pushed
the
chore/replaceUseSafeRefCallback
branch
from
d70ebd8 to
ff74e1b
Compare
dougfabris
added
stat: QA assured
dionisio-bot
bot
added
the
stat: ready to merge
Proposed changes (including videos or screenshots)
Issue(s)
ARCH-1658
Steps to test or reproduce
Further comments
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.