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

Fix SSR entering infinite loop when multiple query hooks update #459

Merged
merged 1 commit into from
Oct 29, 2019

Conversation

kitten
Copy link
Member

@kitten kitten commented Oct 29, 2019

Fix #457

This bug can be replicated by:

  • Having a useQuery hook in a component
  • Either: having a second with the same operation or another hook in the
    component
  • Triggering an update causing the component to rerender

This will cause the component to rerender in SSR.
The useCallback is a noop so some dependencies will trigger,
and this will subsequently trigger another update, which then
enters an infinite loop.

This is because useCallback is a noop in SSR: https://github.com/facebook/react/blob/0f64703edf5970b12a878ea3b5e1e30ef1d71c74/packages/react-dom/src/server/ReactPartialRendererHooks.js#L448-L454

@kitten kitten added the bug 🐛 Oh no! A bug or unintented behaviour. label Oct 29, 2019
This bug can be replicated by:

- Having a `useQuery` hook in a component
- Either: having a second with the same operation or another hook in the
  component
- Triggering an update causing the component to rerender

This will cause the component to rerender in SSR.
The `useCallback` is a noop so some dependencies will trigger,
and this will subsequently trigger another update, which then
enters an infinite loop.
Copy link
Collaborator

@JoviDeCroock JoviDeCroock left a comment

Choose a reason for hiding this comment

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

Did not know about that, that's kinda tricky. Great find!

@kitten kitten merged commit c1b08c8 into master Oct 29, 2019
@kitten kitten deleted the fix/ssr-deadlock branch October 29, 2019 16:15
@parkerziegler
Copy link
Contributor

parkerziegler commented Oct 29, 2019

Wow that's wild. I was actually hitting this yesterday working on a small NextJS app and found that changing the dependency array arbitrarily on a useCallback didn't appear to be affecting when it was re-memoized.

@kitten
Copy link
Member Author

kitten commented Oct 29, 2019

@parkerziegler it really is and really was hard to pin down. I'm totally aware that useMemo and useCallback make no strong guarantees, but I was totally expecting useCallback to still work in most simple cases.

Edit: If anyone has some suggestions on how to improve the current useQuery implementation though, please let me know! It isn't too pretty right now 😅

@parkerziegler
Copy link
Contributor

:thonking-hard: 😅

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🐛 Oh no! A bug or unintented behaviour.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Duplicate useQuery()s in the same component causes "Invariant Violation: Too many re-renders" during SSR
3 participants