Skip to content
This repository has been archived by the owner on Apr 13, 2023. It is now read-only.

Prevent new data re-render attempts during an existing render #3902

Merged
merged 8 commits into from
Apr 3, 2020

Conversation

hwillson
Copy link
Member

@hwillson hwillson commented Mar 27, 2020

As of version 16.13.0, React logs a warning when a function component is updated during another component's render phase (facebook/react#17099). In React Apollo, this warning can be triggered when nesting multiple components that leverage useQuery. To help avoid this, this PR ensures re-render requests to show new data are delayed until an effect hook is run to handle them (since we're then out of the render phase).

Fixes #3863

This commit includes a few small `QueryData` adjustments:

- Replace the `forceUpdate` callback property name with
  `onNewData`, to make its use more clear.
- Create a small public "has SSR started" helper utility that
  can be used by things like the `useQuery` hook to change its
  behavior depending on how it's being used.
As of version 16.13.0, React logs a warning when a function
component is updated during another component's render phase
(facebook/react#17099). In React Apollo,
this warning can be triggered when nesting multiple components that
leverage `useQuery`. To help avoid this, this commit ensures
re-render requests to show new data are delayed until an effect
hook is run to handle them (since we're then out of the render
phase).
The latest `useReducer` API doesn't require an initial value.
@hwillson hwillson requested a review from benjamn March 27, 2020 13:17
@hwillson hwillson added this to Web in Client Team Mar 27, 2020
@hwillson hwillson self-assigned this Mar 27, 2020
Copy link

@JSONRice JSONRice left a comment

Choose a reason for hiding this comment

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

Looks good. Just left one comment for my own education.

packages/hooks/src/data/QueryData.ts Outdated Show resolved Hide resolved
packages/hooks/src/utils/useBaseQuery.ts Show resolved Hide resolved
Copy link
Member

@benjamn benjamn left a comment

Choose a reason for hiding this comment

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

One note about the downsides of ?. syntax, but otherwise this looks good!

packages/hooks/src/data/QueryData.ts Outdated Show resolved Hide resolved
@hwillson hwillson merged commit 678556b into release-3.x Apr 3, 2020
@hwillson hwillson deleted the issue-3863 branch April 3, 2020 10:19
hwillson added a commit to apollographql/apollo-client that referenced this pull request Apr 3, 2020
As of version 16.13.0, React logs a warning when a function
component is updated during another component's render phase
(facebook/react#17099). In Apollo Client this warning can be
triggered when nesting multiple components that leverage
`useQuery`. To help avoid this, this commit ensures re-render
requests to show new data are delayed until an effect hook is
run to handle them (since we're then out of the render phase).

These changes were originally implemented in
apollographql/react-apollo#3902.
hwillson added a commit to apollographql/apollo-client that referenced this pull request Apr 3, 2020
As of version 16.13.0, React logs a warning when a function
component is updated during another component's render phase
(facebook/react#17099). In Apollo Client this warning can be
triggered when nesting multiple components that leverage
`useQuery`. To help avoid this, this commit ensures re-render
requests to show new data are delayed until an effect hook is
run to handle them (since we're then out of the render phase).

These changes were originally implemented in
apollographql/react-apollo#3902.
@Vadorequest
Copy link

This creates a SSR regression when used by Next.js framework.

lfades/next-with-apollo#126 similar regression (SSR) when upgrading from v4 to v5 (which uses @apollo/react-ssr)

Tested on UnlyEd/next-right-now#27

I don't think it's gonna be blocking (for me) but others could run into similar issue. If so, I believe following lfades/next-with-apollo#126 (comment) is the way to go, but I'm about to try and will confirm it.

hwillson added a commit that referenced this pull request Apr 13, 2020
As of version 16.13.0, React logs a warning when a function component
is updated during another component's render phase
(facebook/react#17099). PR #3902 addresses some of this, but doesn't
quite catch all scenarios where this can occur. These changes take
things further, ensuring that React Apollo components can't force
update during a render phase.
hwillson added a commit that referenced this pull request Apr 13, 2020
As of version 16.13.0, React logs a warning when a function component
is updated during another component's render phase
(facebook/react#17099). PR #3902 addresses some of this, but doesn't
quite catch all scenarios where this can occur. These changes take
things further, ensuring that React Apollo components can't force
update during a render phase.
hwillson added a commit that referenced this pull request Apr 14, 2020
As of version 16.13.0, React logs a warning when a function component
is updated during another component's render phase
(facebook/react#17099). PR #3902 addresses some of this, but doesn't
quite catch all scenarios where this can occur. These changes take
things further, ensuring that React Apollo components can't force
update during a render phase.
hwillson added a commit to apollographql/apollo-client that referenced this pull request Apr 16, 2020
As of version 16.13.0, React logs a warning when a function
component is updated during another component's render phase
(facebook/react#17099). In Apollo Client this warning can be
triggered when nesting multiple components that leverage
`useQuery`. To help avoid this, this commit ensures re-render
requests to show new data are delayed until an effect hook is
run to handle them (since we're then out of the render phase).

These changes were originally implemented in
apollographql/react-apollo#3902.
hwillson added a commit to apollographql/apollo-client that referenced this pull request Apr 16, 2020
As of version 16.13.0, React logs a warning when a function
component is updated during another component's render phase
(facebook/react#17099). In Apollo Client this warning can be
triggered when nesting multiple components that leverage
`useQuery`. To help avoid this, this commit ensures re-render
requests to show new data are delayed until an effect hook is
run to handle them (since we're then out of the render phase).

These changes were originally implemented in
apollographql/react-apollo#3902.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
No open projects
Client Team
In progress
Development

Successfully merging this pull request may close these issues.

None yet

4 participants