-
Notifications
You must be signed in to change notification settings - Fork 47.3k
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
Memoized child of Suspense component doesn't update when Context updates. #17356
Comments
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 ( |
@gaearon can you take a look at this when you get a chance? |
It appears to relate to using Here's the same example but with 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, 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 With this being said, if the |
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: #18572 (comment) |
https://codesandbox.io/s/react-suspense-context-bug-cq2xq Probably another example of this issue. |
Possible fix: #19216 |
* Add a failing test for legacy Suspense blocking context updates in memo * Add more test case coverage for variations of #17356 * Don't bailout after Suspending in Legacy Mode Co-authored-by: Tharuka Devendra <[email protected]>
Do you want to request a feature or report a bug?
bug (I think?)
What is the current behavior?
When using a memoized functional component (
MemoizedChild
in above example) in conjunction withContext
as a child of aReact.Suspense
component, there seems to be a bug in whichMemoizedChild
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 flipsSuspense
to the fallback state and when the promise resolvesMemoizedChild
's state is not updated with the proper context value because (I'm assuming) the memoized value ofMemoizedChild
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?
The text was updated successfully, but these errors were encountered: