[SuspenseList] Fix bugs with dropped Promises #17082
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
For non-hidden modes, this boundary should commit so this shouldn't be needed but the nested boundary can make a second pass which forces these to be recreated without resuspending. In this case, the outer list assumes that it can collect the inner promises to still rerender if needed.
This means that we always rerender any nested SuspenseLists together. This bug looks similar to the previous one but is not based on the lack of retry but that the retry only happens on the outer boundary but the inner doesn't get a retry ping since it didn't know about its own promise after the second pass.
Details of bundled changes.Comparing: 75955bf...654572d react-art
react-dom
react-native-renderer
react-test-renderer
react-reconciler
|
acdlite
approved these changes
Oct 14, 2019
Collaborator
acdlite
left a comment
acdlite
left a comment
There was a problem hiding this comment.
Re: the first fix, maybe we could try the thing where instead of attaching the promise to the inner Suspense boundary, it always gets attached to the outer SuspenseList, so that there's nothing to transfer if the inner boundary does another pass. Would that have worked in this case?
Collaborator
Author
|
It’s tricky to do that in throwException because you have to know the context at every level and you don’t know the context of the shadowed levels. Otherwise you don’t know whether to continue bubbling or not. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This includes two similar looking bug fixes to SuspenseList.
We must transfer any pending promises from inner boundary to list. For non-hidden modes, this boundary should commit so this shouldn't be needed but the nested boundary can make a second pass which forces these to be recreated without resuspending. In this case, the outer list assumes
that it can collect the inner promises to still rerender if needed.
Otherwise, the Promise never retries anything.
We must also propagate suspense "context" change to nested SuspenseLists. This bug looks similar to the previous one but is not based on the lack of retry but that the retry only happens on the outer boundary but the inner doesn't get a retry ping since it didn't know about its own promise after the second pass. So it bails out.