RocketChat / RocketChat/Rocket.Chat

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

Open Beginner friendly
#40,589 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

type: bug
Dominant language
TypeScript
Stars
46.1k
Forks
13.9k
Avg merge
3d 3h
Merged PRs (30d)
130

Description

Description

In apps/meteor/client/views/admin/rooms/RoomsTable.tsx, a state setter (setCurrent(0)) is called inside a useMemo callback to reset pagination when the search text changes. Calling state setters inside useMemo is a side effect during render, which React explicitly prohibits — memos must be pure computations.

Affected File

apps/meteor/client/views/admin/rooms/RoomsTable.tsx

Problematic Code

const query = useDebouncedValue(
    useMemo(() => {
        if (searchText !== prevRoomFilterText.current) {
            setCurrent(0); //  side effect inside useMemo
        }
        return { ... };
    }, [searchText, sortBy, ...]),
    500,
);

Why It's Wrong

  • useMemo must be a pure computation with no side effects
  • Calling setCurrent here triggers a state update during render
  • In React Strict Mode, useMemo can be invoked multiple times, potentially firing setCurrent(0) more than once
  • prevRoomFilterText is updated in a useEffect which runs after render, creating a timing window where the condition fires incorrectly on intermediate re-renders

Proposed Fix

Move the pagination reset into a useEffect watching searchText, and remove the side effect from the memo:

useEffect(() => {
    setCurrent(0);
    prevRoomFilterText.current = searchText;
}, [searchText, setCurrent]);

And simplify the memo to a pure computation:

const query = useDebouncedValue(
    useMemo(() => ({
        filter: searchText || '',
        sort: `{ "${sortBy}": ${sortDirection === 'asc' ? 1 : -1} }`,
        count: itemsPerPage,
        offset: current,
        types: ...,
    }), [searchText, sortBy, sortDirection, itemsPerPage, current, roomFilters.types]),
    500,
);

Notes

Verified on develop branch. Checked UsersTable.tsx and ChannelsTable.tsx — neither has this pattern. Issue is isolated to RoomsTable.tsx.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with apps/meteor/client/views/admin/rooms/RoomsTable.tsx and inspect the useMemo, searchText handling, and related useEffect. Move the pagination reset out of render-time memo evaluation while preserving the debounced query behavior. Done means the memo is a pure computation and changing search text still resets pagination correctly.

Written by the indexing model from the issue text.

Assessment

Tech stack
react, typescript
Domain
frontend
Issue type
Bug
Difficulty
2/5
Estimated time
1-3 hours
Activity status
Quiet
Clarity
Clearly specified
Newbie friendliness
76/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.