refactor: split useRoomNavigation and drop InteractionManager from initial jump params - #7682
Conversation
… openThread/openRoom services Rename the hook to useJumpToMessage to match the CONTEXT.md Positional State row. Move Thread and Room opening into plain async services with explicit deps. Isolate route-param consumption in a non-exported helper hook so the generation counter has a single owner. Behavior and public surface unchanged; the three existing suites pass with only import renames. NATIVE-1550
…ct instead of InteractionManager
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (10)
Included review availability: Your plan provides up to 8 included reviews per hour; 7 remain after this review. 📜 Recent review details🧰 Additional context used📓 Path-based instructions (3)Format JavaScript and TypeScript code with Oxfmt using the repository configuration: tabs, single quotes, 130-character width, no trailing commas, omitted arrow-function parentheses where possible, and same-line brackets.📄 CodeRabbit inference engine (CLAUDE.md) Files:
Use descriptive names for functions, variables, and classes that clearly convey their purpose Write comments that explain the 'why' behind code decisions, not the 'what' Keep functions small and focused on a single responsibility Use const...📄 CodeRabbit inference engine (AGENTS.md) Files:
Use TypeScript for type safety; add explicit type annotations to function parameters and return types Prefer interfaces over type aliases for defining object shapes in TypeScript Use enums for sets of related constants rather than magic str...📄 CodeRabbit inference engine (AGENTS.md) Files:
🔇 Additional comments (8)
WalkthroughThe pull request renames ChangesRoom jump navigation
Priority: ⬇️ Low Estimated code review effort: 4 (Complex) | ~45 minutes Change: Refactor Sequence Diagram(s)sequenceDiagram
participant useJumpToMessage
participant openRoom
participant openThread
participant RoomView
useJumpToMessage->>openRoom: Open a room message target
openRoom->>RoomView: Resolve room and navigate
useJumpToMessage->>openThread: Open a thread target
openThread->>RoomView: Fetch thread name and navigate
Suggested labels: Merge Risk: ⚪ Minimal · up to No unresolved merge-blocking risk remains from this navigation refactor. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1⚔️ Resolve merge conflicts 💡
Warning Errors were encountered while retrieving linked issues. Errors (1)
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 |
Proposed changes
Two follow-ups to the RoomView hooks migration, both scoped to how RoomView reaches a target Message or Thread.
Split
useRoomNavigationintouseJumpToMessageand plain services. The hook is renamed to match the Positional State row in CONTEXT.md. Opening a Thread or another Room moves intoopenThreadandopenRoom, async services with explicit dependencies. Consumption of the initial route params (jumpToMessageId,jumpToThreadId) lives in a non-exported helper so the jump generation counter has one owner. Public surface (onThreadMessagesLoaded,onThreadPress,jumpToMessageByUrl) is unchanged.Consume initial route params in a mount effect instead of
InteractionManager.runAfterInteractions. The gate never waited for the navigation transition: react-navigation registers no interaction handles, and on this React Native versionInteractionManageris the stub whoserunAfterInteractionsissetImmediate. The API is deprecated and starts warning at runtime in 0.84. The helper now reads the initial Message target first and the once-only Thread target second in a single mount effect, keeping the message-before-thread guard. The jump starts one tick earlier than before. In-flight jumps still settle after unmount, as before; the generation counter is not bumped on unmount.useRoomInit,useRoomSubscriptionandJoinCodekeep their ownInteractionManagerusage and are not touched here.Issue(s)
https://rocketchat.atlassian.net/browse/NATIVE-1550
https://rocketchat.atlassian.net/browse/NATIVE-1551
How to test or reproduce
Each step starts from a Room that is not currently open.
Screens: RoomView, Thread view.
Screenshots
Device runs on iPhone 16 Pro simulator and Pixel API 34 emulator, both against mobile.qa. Each recording starts with the target Room not mounted.
iOS, message target into a not-mounted Room. A permalink to a message in another Room is tapped; the Room mounts with the target highlighted.
ios-checkA-messagelink.mp4
iOS, message target that is a Thread reply. The Room mounts, the reply is resolved, and exactly one Thread screen opens; back returns to the Room.
ios-checkBC-threadreply.mp4
Search result into the current Room, both platforms. Same highlight as before the change.
Not covered on device: the
jumpToThreadIdroute param. It is only produced by therocketchat://roomdeep link, which does not navigate on this Android emulator (the intent reaches the activity, the JS side never navigates; the auth deep link behaves the same). The Android not-mounted-Room run was also blocked by the test environment's login. Thread-param ordering is pinned by the lifecycle and composed suites instead.Types of changes
Checklist
Further comments
The lifecycle and composed suites no longer spy on
InteractionManager. Tests that only passed because a mocked scheduler was never released were removed rather than rewritten, since after this change there is no deferred task to cancel and an "after unmount" assertion would only have exercised its own never-resolving mocks. The contract that pending asynchronous work keeps settling after unmount is still pinned by its existing test.Summary by CodeRabbit
New Features
Bug Fixes