Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

React useThreads incorrect usage of useSyncExternalStoreWithSelector triggers infinite re-renders #1442

Closed
colindjk opened this issue Jan 31, 2024 · 1 comment
Labels
bug Something isn't working

Comments

@colindjk
Copy link

Describe the bug

The useThreads component passes a new selector to useSyncExternalStoreWithSelector on each render. This function is used here packages/liveblocks-react/src/comments/CommentsRoom.tsx line 488.

This makes it so that every time useThreads is called, the referenced threads binding will be a new object.

To Reproduce

const { threads } = useThreads()
const [visibleThreads, setVisibleThreads] = useState([])

// this particular case could be avoided with useMemo but not all cases can be solved that way. 
useEffect(() => {
  setVisibleThreads(threads.filter(thread => thread.metadata.visible))
}, [threads])

The re-render triggered by the setVisibleThreads will trigger an infinite re-render since threads will always have a new reference.

Workaround
Use deep equals for any state depending on threads.

Fix

To fix this the following needs to be updated to use memoized functions:

    return useSyncExternalStoreWithSelector(
      store.subscribe,

      // Change the below to be memoized / useCallback functions. 
      () => store.getThreads(),
      () => store.getThreads(),
      (state) => {

Environment (please complete the following information):

@colindjk colindjk added bug Something isn't working triage needed The issue needs to be reviewed by the team labels Jan 31, 2024
@GuillaumeSalles
Copy link
Contributor

Solved in [1.10.0-beta2](https://github.com/liveblocks/liveblocks/releases/tag/v1.10.0-beta2).

If it does not solve your issue, don't hesitate to leave a comment and I'll reopen it. Thank you!

@GuillaumeSalles GuillaumeSalles removed the triage needed The issue needs to be reviewed by the team label Feb 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants