Skip to content

fix: replace module-scoped ref registry with instance-local ref in UIKit Overflow#7123

Open
deepak0x wants to merge 1 commit intoRocketChat:developfrom
deepak0x:fix/overflow-instance-local-ref
Open

fix: replace module-scoped ref registry with instance-local ref in UIKit Overflow#7123
deepak0x wants to merge 1 commit intoRocketChat:developfrom
deepak0x:fix/overflow-instance-local-ref

Conversation

@deepak0x
Copy link
Copy Markdown
Contributor

@deepak0x deepak0x commented Apr 10, 2026

The Overflow component was storing refs in a module-scoped touchable map keyed by blockId. Two issues with that:

  1. The map only ever grew. Refs added during a session were never cleaned up, so every blockId ever rendered stayed in memory until the app shut down.
  2. When blockId was missing or repeated across instances, multiple Overflow components ended up sharing the same ref, so the Popover would sometimes anchor to the wrong button.

Switched to an instance-local useRef so the ref's lifecycle is tied to the component. I kept the as React.RefObject<any> cast that was already in the original code, since Touch's ref is a RectButton instance and Popover.from strictly wants a RefObject<View>. Same workaround, just no longer global.

Issue(s)

Closes #7095

How to test or reproduce

The bug shows up when multiple Overflow components are rendered on the same screen (e.g. UIKit messages with more than one overflow menu, or when an Overflow is re-rendered with an empty blockId). Tapping the kebab on one would sometimes pop the menu over a different button. With this fix each instance has its own ref, so the popover always anchors correctly.

Types of changes

  • Bugfix (non-breaking change which fixes an issue)
  • Improvement (non-breaking change which improves a current function)
  • New feature (non-breaking change which adds functionality)
  • Documentation update (if none of the other choices apply)

Checklist

  • I have read the CONTRIBUTING doc
  • I have signed the CLA
  • Lint and unit tests pass locally with my changes
  • I have added tests that prove my fix is effective or that my feature works (if applicable)
  • I have added necessary documentation (if applicable)
  • Any dependent changes have been merged and published in downstream modules

Summary by CodeRabbit

  • Refactor
    • Simplified internal reference management for improved component performance and maintainability. No user-facing changes.

…Kit Overflow

Refs were stored in a module-level map keyed by blockId, which caused two
problems:

- Memory leak: refs were never cleaned up, so every blockId seen during
  the app session accumulated in the map for the lifetime of the process.
- Wrong popover anchor: when blockId was empty or shared across instances,
  multiple Overflow components ended up sharing the same ref, causing the
  Popover to anchor to the wrong element.

Switched to a useRef inside the component so the ref lifecycle is tied to
the instance.

Closes RocketChat#7095
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Apr 10, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 4759b7bf-88f7-4e73-8053-040edd1e5499

📥 Commits

Reviewing files that changed from the base of the PR and between cb0feee and c06a8f6.

📒 Files selected for processing (1)
  • app/containers/UIKit/Overflow.tsx
📜 Recent review details
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{js,jsx,ts,tsx,json}

📄 CodeRabbit inference engine (CLAUDE.md)

Configure Prettier with tabs, single quotes, 130 character width, no trailing commas, arrow parens avoid, and bracket same line

Files:

  • app/containers/UIKit/Overflow.tsx
**/*.{js,jsx,ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

Use ESLint with @rocket.chat/eslint-config base configuration including React, React Native, TypeScript, and Jest plugins

Files:

  • app/containers/UIKit/Overflow.tsx
**/*.{ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

Use TypeScript with strict mode enabled and configure baseUrl to app/ for import resolution

**/*.{ts,tsx}: 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 strings or numbers

Files:

  • app/containers/UIKit/Overflow.tsx
app/containers/**/*.{ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

Place reusable UI components in app/containers/ directory

Files:

  • app/containers/UIKit/Overflow.tsx
**/*.{js,ts,jsx,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

**/*.{js,ts,jsx,tsx}: 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 by default, let when reassignment is needed, and avoid var
Prefer async/await over .then() chains for handling asynchronous operations
Use explicit error handling with try/catch blocks for async operations
Avoid deeply nested code; refactor complex logic into helper functions

Files:

  • app/containers/UIKit/Overflow.tsx
🧠 Learnings (2)
📚 Learning: 2026-04-07T17:49:17.538Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat.ReactNative PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-07T17:49:17.538Z
Learning: Applies to app/stacks/**/*.{ts,tsx} : Use React Navigation 7 for navigation with stacks for InsideStack (authenticated), OutsideStack (login/register), MasterDetailStack (tablets), and ShareExtensionStack

Applied to files:

  • app/containers/UIKit/Overflow.tsx
📚 Learning: 2026-04-07T17:49:17.538Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat.ReactNative PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-04-07T17:49:17.538Z
Learning: Support React 19, React Native 0.79, and Expo 53

Applied to files:

  • app/containers/UIKit/Overflow.tsx
🔇 Additional comments (2)
app/containers/UIKit/Overflow.tsx (2)

1-1: Import update looks good.

Adding useRef here cleanly supports the instance-local ref lifecycle introduced in this fix.


48-65: Core ref-localization fix is solid.

Moving to a component-local touchableRef correctly isolates instances and removes the prior shared-ref/leak behavior while preserving existing Popover anchoring usage.


Walkthrough

The Overflow component's ref storage mechanism was refactored from a module-scoped map keyed by blockId to an instance-local ref created via useRef. This eliminates ref memory leaks and prevents ref collision across multiple component instances.

Changes

Cohort / File(s) Summary
Ref Storage Refactor
app/containers/UIKit/Overflow.tsx
Replaced module-level touchable map with instance-local useRef<View>(null), removing block ID derivation and lazy initialization logic to prevent memory leaks and ref collisions.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Suggested labels

type: bug

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main change: replacing a module-scoped ref registry with an instance-local ref in the UIKit Overflow component.
Linked Issues check ✅ Passed The pull request successfully implements the fix required by issue #7095: replacing the module-scoped touchable map with an instance-local useRef, eliminating ref leaks and preventing incorrect popover anchoring.
Out of Scope Changes check ✅ Passed All changes are focused on the Overflow component's ref management and directly address the objectives outlined in issue #7095; no out-of-scope modifications detected.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix: replace module-scoped ref registry with instance-local ref in UIKit Overflow component

1 participant