Skip to content

fix(meteor): useMemo side-effect in RoomsTable#40617

Open
TasfinMahmud wants to merge 2 commits into
RocketChat:developfrom
TasfinMahmud:fix/40589-roomstable-usememo-sideeffect
Open

fix(meteor): useMemo side-effect in RoomsTable#40617
TasfinMahmud wants to merge 2 commits into
RocketChat:developfrom
TasfinMahmud:fix/40589-roomstable-usememo-sideeffect

Conversation

@TasfinMahmud
Copy link
Copy Markdown

@TasfinMahmud TasfinMahmud commented May 19, 2026

Fixes #40589 by extracting the setCurrent(0) side effect from useMemo into useEffect where state mutations belong, ensuring pure computation and respecting React's rendering contract.

Summary by CodeRabbit

Release Notes

  • Bug Fixes
    • Fixed pagination behavior in the admin rooms table to properly reset to the first page when filtering by search text.

Review Change Stack

@TasfinMahmud TasfinMahmud requested a review from a team as a code owner May 19, 2026 20:31
@dionisio-bot
Copy link
Copy Markdown
Contributor

dionisio-bot Bot commented May 19, 2026

Looks like this PR is not ready to merge, because of the following issues:

  • This PR is missing the 'stat: QA assured' label
  • This PR is missing the required milestone or project

Please fix the issues and try again

If you have any trouble, please check the PR guidelines

@changeset-bot
Copy link
Copy Markdown

changeset-bot Bot commented May 19, 2026

⚠️ No Changeset found

Latest commit: fc8f3f3

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 19, 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: 0727cb03-867d-4b26-a6f8-72a73a792b64

📥 Commits

Reviewing files that changed from the base of the PR and between 2d39044 and ae69f88.

📒 Files selected for processing (1)
  • apps/meteor/client/views/admin/rooms/RoomsTable.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
  • apps/meteor/client/views/admin/rooms/RoomsTable.tsx

Walkthrough

RoomsTable pagination behavior is refactored to reset pagination state when searchText changes. The conditional setCurrent(0) call is removed from the useMemo query builder, the dependency array is updated, and a dedicated useEffect is added to properly reset pagination and track filter changes.

Changes

RoomsTable Pagination Side Effect Fix

Layer / File(s) Summary
Pagination reset timing correction
apps/meteor/client/views/admin/rooms/RoomsTable.tsx
Removes setCurrent(0) side effect from useMemo query builder (lines 42–46), updates the memoization dependency array to remove setCurrent (line 58), and adds a dedicated useEffect (lines 74–78) that properly resets pagination when searchText changes and updates the previous filter text tracker.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

🚥 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 clearly and specifically identifies the main change: fixing a side effect in useMemo in RoomsTable component.
Linked Issues check ✅ Passed The PR successfully implements all objectives from issue #40589: extracts the setCurrent(0) side effect from useMemo into a useEffect, ensures pure computation in useMemo, and updates dependencies accordingly.
Out of Scope Changes check ✅ Passed All changes are directly scoped to fixing the identified React rendering contract violation in RoomsTable.tsx with no extraneous modifications.
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.

Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1 issue found across 3 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name=".changeset/fix-ip-whitelist-trim.md">

<violation number="1" location=".changeset/fix-ip-whitelist-trim.md:5">
P2: Changeset describes unrelated IP whitelist fix instead of the PR's RoomsTable useMemo fix</violation>
</file>

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

Comment thread .changeset/fix-ip-whitelist-trim.md Outdated
"@rocket.chat/meteor": patch
---

fix: trim whitespace from IP whitelist entries in failed-login protection
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: Changeset describes unrelated IP whitelist fix instead of the PR's RoomsTable useMemo fix

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At .changeset/fix-ip-whitelist-trim.md, line 5:

<comment>Changeset describes unrelated IP whitelist fix instead of the PR's RoomsTable useMemo fix</comment>

<file context>
@@ -0,0 +1,5 @@
+"@rocket.chat/meteor": patch
+---
+
+fix: trim whitespace from IP whitelist entries in failed-login protection
</file context>

@TasfinMahmud TasfinMahmud force-pushed the fix/40589-roomstable-usememo-sideeffect branch from 2d39044 to ae69f88 Compare May 19, 2026 22:13
@coderabbitai coderabbitai Bot removed the type: bug label May 19, 2026
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.

RoomsTable: Side effect (setCurrent) called inside useMemo violates React rendering contract

1 participant