Try rendering again if a timed out tree receives an update #13921
Conversation
| @@ -35,19 +35,17 @@ function assertYieldsWereCleared(root) { | |||
| } | |||
|
|
|||
| export function toFlushAndYield(root, expectedYields) { | |||
| assertYieldsWereCleared(root); | |||
There was a problem hiding this comment.
I moved these outside because captureAssertion was swallowing the stack trace.
| currentFallbackChildFragment, | ||
| currentFallbackChildFragment.pendingProps, | ||
| currentPrimaryChildFragment, | ||
| currentPrimaryChildFragment.pendingProps, |
There was a problem hiding this comment.
This is a fix for an oopsies that could have been its own PR, but it was required to get the new test I wrote to pass. I can split it out if I need to.
| ) { | ||
| // The primary children have pending work. Use the normal path | ||
| // to attempt to render the primary children again. | ||
| return updateSuspenseComponent( |
There was a problem hiding this comment.
This is the main fix here.
| @@ -23,3 +24,17 @@ export type SuspenseState = {| | |||
| // `didTimeout` because it's not set unless the boundary actually commits. | |||
| timedOutAt: ExpirationTime, | |||
| |}; | |||
|
|
|||
| export function shouldCaptureSuspense( | |||
There was a problem hiding this comment.
I split this out because it was getting hard to follow all the nested branches in the throwException function
| let nextState = workInProgress.memoizedState; | ||
| if (currentState === null) { | ||
| const currentState: SuspenseState | null = | ||
| current !== null ? current.memoizedState : null; |
There was a problem hiding this comment.
I believe this will fix the issue in www, haven't confirmed yet. I'll try on my sandbox shortly.
Found a bug related to suspending inside an already mounted tree. While investigating this I noticed we really don't have much coverage of suspended updates. I think this would greatly benefit from some fuzz testing; still haven't thought of a good test case, though.
acdlite
force-pushed
the
suspendonupdate
branch
from
55787b4
to
8615f1c
Compare
|
@sebmarkbage Hmm I remember hitting this issue with context and shrugging it off, not state though. (Oh, and context should just work now that the timed out children remain in the tree. Should add a test for that as a follow up.) |
…13921) Found a bug related to suspending inside an already mounted tree. While investigating this I noticed we really don't have much coverage of suspended updates. I think this would greatly benefit from some fuzz testing; still haven't thought of a good test case, though.
Found a bug related to suspending inside an already mounted tree. While investigating this I noticed we really don't have much coverage of suspended updates — nearly all are testing the initial mount. I think this would greatly benefit from some fuzz testing; still haven't thought of a good test case, though.