Do you want to request a feature or report a bug?
bug (I think?)
What is the current behavior?
const [value, setValue] = useState("default");
return (
<div className="App">
<input value={value} onChange={e => setValue(e.target.value)} />
<div>
<Value.Provider value={value}>
<Suspense fallback={<div>loading</div>}>
<MemoizedChild />
</Suspense>
</Value.Provider>
</div>
</div>
)
When using a memoized functional component (MemoizedChild in above example) in conjunction with Context as a child of a React.Suspense component, there seems to be a bug in which MemoizedChild does not update when the context it uses changes. For the full example, see my codesandbox below.
In the codesandbox, if you change the value of the input, the new value is provided to the context which causes the hook used in MemoizedChild (useValue) to throw a promise. This flips Suspense to the fallback state and when the promise resolves MemoizedChild's state is not updated with the proper context value because (I'm assuming) the memoized value of MemoizedChild is the one that contained the previous context value and technically no props have changed, so that makes sense why it wouldn't have updated. However, this seems like it would be unexpected behavior.
https://codesandbox.io/s/react-suspense-maybe-bug-sznbk
What is the expected behavior?
I would expect that MemoizedChild would be re-rendered with the new provided value.
Which versions of React, and which browser / OS are affected by this issue? Did this work in previous versions of React?
I'm assuming all of them that contain Suspense and memo. So, since 16.8?
Thanks for the report @jcarroll2007, this does look like a bug. I reproduced with the experimental build as well and also verified that this only affects legacy mode (ReactDOM.render), Concurrent and Blocking Mode both behave as expected.
@gaearon can you take a look at this when you get a chance?
It appears to relate to using useContext() and throwing in the same component.
Here's the same example but with MemoizedChild split into two components - one with the useContext() hook, and a child inside it which throws.
https://codesandbox.io/s/react-suspense-maybe-bug-c62nh
This behaves as expected.
Would anyone like to turn this into a failing test? This will get us closer to finding a fix.
@gaearon can I take this up? I'll start with the failing test to figure out the bug!
Sure!
Hi @gaearon, I've been looking into this a bit (first time exploring the react code base, so sorry if i've misunderstood anything) and this is what I've understood so far for the reproduction of the bug given in the first sandbox (https://codesandbox.io/s/react-suspense-maybe-bug-sznbk):
When the promise resolves after being thrown, didReceiveUpdate is false for the MemoizedChild when it is processed in updateFunctionComponent during the beginWork phase. This makes sense since from what I understand, didReceiveUpdate is conditional on a change of props or a change of context (might have missed some other cases) but neither of these happened after the promise resolved, hence it is correct that didReceiveUpdate is false. But due to didReceiveUpdate being false for the MemoizedChild, it means reconcileChildren is never called which is what would normally update the value (from what I understand).
As @overlookmotel pointed out, separating the context and throwing promise in two different components fixes the issue (https://codesandbox.io/s/react-suspense-maybe-bug-c62nh), but I believe in this case it is fixed because ChildOfMemoizedChild receives different props (the old version of resource which throws the promise and the new version which returns the value as intended, and as a result didReceiveUpdate is true and hence works as intended.
With this being said, if the didReceiveUpdate being false is the issue, I'm not entirely sure how to fix it since it seems more of a design decision than a bug to be able to recognise that a change occurred in this case.
We need a failing test case to look further.
I checked the test case submitted by @tsdevendra1 and this is a bug in legacy mode Suspense: https://github.com/facebook/react/pull/18572#issuecomment-612264828
https://codesandbox.io/s/react-suspense-context-bug-cq2xq
Probably another example of this issue.
Possible fix: https://github.com/facebook/react/pull/19216
Most helpful comment
Thanks for the report @jcarroll2007, this does look like a bug. I reproduced with the experimental build as well and also verified that this only affects legacy mode (
ReactDOM.render), Concurrent and Blocking Mode both behave as expected.