Temporarily disable suspending during work loop #30762
Merged
Conversation
This file contains 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
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
facebook-github-bot
added
CLA Signed
React Core Team
acdlite
changed the title
acdlite
changed the title
acdlite
force-pushed
the
temporarily-disable-sibling-prerendering
branch
from
81171c0
to
0e6571f
Compare
acdlite
force-pushed
the
temporarily-disable-sibling-prerendering
branch
2 times, most recently
from
24e1c11
to
0e6571f
Compare
|
Comparing: b57d282...9759283 Critical size changesIncludes critical production bundles, as well as any change greater than 2%:
Significant size changesIncludes any change greater than 0.2%: Expand to show |
rickhanlonii
approved these changes
Aug 20, 2024
| @@ -558,6 +558,7 @@ describe('ReactUse', () => { | |||
| } | |||
| }); | |||
|
|
|||
| // @gate !enableSiblingPrerendering | |||
There was a problem hiding this comment.
Would it help if I wrote the tests for when this is on?
There was a problem hiding this comment.
I have some more later in the stack, but perhaps I'll move a few into this PR
| // TODO: Suspending the work loop during the render phase is currently | ||
| // not compatible with sibling prerendering. We will add this optimization | ||
| // back in a later step. | ||
| enableSuspendingDuringWorkLoop: !featureFlags.enableSiblingPrerendering, |
There was a problem hiding this comment.
Did you mean to gate the other tests with this flag, or are you going to use it later?
There was a problem hiding this comment.
ah yeah I changed them in a later step, will update
acdlite
force-pushed
the
temporarily-disable-sibling-prerendering
branch
2 times, most recently
from
1d093ad
to
b91b0ef
Compare
acdlite
force-pushed
the
temporarily-disable-sibling-prerendering
branch
from
b91b0ef
to
02a280b
Compare
This is a refactor of the fix in facebook#27505. When a transition update is scheduled by a popstate event, (i.e. a back/ forward navigation) we attempt to render it synchronously even though it's a transition, since it's likely the previous page's data is cached. In facebook#27505, I changed the implementation so that it only "upgrades" the priority of the transition for a single attempt. If the attempt suspends, say because the data is not cached after all, from then on it should be treated as a normal transition. But it turns out facebook#27505 did not work as intended, because it relied on marking the root with pending synchronous work (root.pendingLanes), which was never cleared until the popstate update completed. The test scenarios I wrote accidentally worked for a different reason related to suspending the work loop, which I'm currently in the middle of refactoring.
`use` has an optimization where in some cases it can suspend the work loop during the render phase until the data has resolved, rather than unwind the stack and lose context. However, the current implementation is not compatible with sibling prerendering. So I've temporarily disabled it until the sibling prerendering has been refactored. We will add it back in a later step.
acdlite
force-pushed
the
temporarily-disable-sibling-prerendering
branch
from
02a280b
to
f72bdce
Compare
github-actions bot
pushed a commit
that referenced
this pull request
Sep 4, 2024
### Based on - #30761 - #30759 --- `use` has an optimization where in some cases it can suspend the work loop during the render phase until the data has resolved, rather than unwind the stack and lose context. However, the current implementation is not compatible with sibling prerendering. So I've temporarily disabled it until the sibling prerendering has been refactored. We will add it back in a later step. DiffTrain build for [8b4c54c](8b4c54c)
github-actions bot
pushed a commit
that referenced
this pull request
Sep 4, 2024
### Based on - #30761 - #30759 --- `use` has an optimization where in some cases it can suspend the work loop during the render phase until the data has resolved, rather than unwind the stack and lose context. However, the current implementation is not compatible with sibling prerendering. So I've temporarily disabled it until the sibling prerendering has been refactored. We will add it back in a later step. DiffTrain build for commit 8b4c54c.
This was referenced Sep 5, 2024
gnoff
pushed a commit
to vercel/next.js
that referenced
this pull request
Sep 12, 2024
**breaking change for canary users: Bumps peer dependency of React from `19.0.0-rc-7771d3a7-20240827` to `19.0.0-rc-94e652d5-20240912`** [diff facebook/react@7771d3a7...94e652d5](facebook/react@7771d3a...94e652d) <details> <summary>React upstream changes</summary> - facebook/react#30952 - facebook/react#30950 - facebook/react#30946 - facebook/react#30934 - facebook/react#30947 - facebook/react#30945 - facebook/react#30938 - facebook/react#30936 - facebook/react#30879 - facebook/react#30888 - facebook/react#30931 - facebook/react#30930 - facebook/react#30832 - facebook/react#30929 - facebook/react#30926 - facebook/react#30925 - facebook/react#30905 - facebook/react#30900 - facebook/react#30910 - facebook/react#30906 - facebook/react#30899 - facebook/react#30919 - facebook/react#30708 - facebook/react#30907 - facebook/react#30897 - facebook/react#30896 - facebook/react#30895 - facebook/react#30887 - facebook/react#30889 - facebook/react#30893 - facebook/react#30892 - facebook/react#30891 - facebook/react#30882 - facebook/react#30881 - facebook/react#30870 - facebook/react#30849 - facebook/react#30878 - facebook/react#30865 - facebook/react#30869 - facebook/react#30875 - facebook/react#30800 - facebook/react#30762 - facebook/react#30831 - facebook/react#30866 - facebook/react#30853 - facebook/react#30850 - facebook/react#30847 - facebook/react#30842 - facebook/react#30837 - facebook/react#30848 - facebook/react#30844 - facebook/react#30839 - facebook/react#30802 - facebook/react#30841 - facebook/react#30827 - facebook/react#30826 - facebook/react#30825 - facebook/react#30824 - facebook/react#30840 - facebook/react#30838 - facebook/react#30836 - facebook/react#30819 - facebook/react#30816 - facebook/react#30814 - facebook/react#30813 - facebook/react#30812 - facebook/react#30811 </details> --------- Co-authored-by: vercel-release-bot <infra+release@vercel.com>
mofeiZ
added a commit
that referenced
this pull request
Oct 11, 2024
### Based on - #30761 - #30759 --- `use` has an optimization where in some cases it can suspend the work loop during the render phase until the data has resolved, rather than unwind the stack and lose context. However, the current implementation is not compatible with sibling prerendering. So I've temporarily disabled it until the sibling prerendering has been refactored. We will add it back in a later step. [ghstack-poisoned]
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Based on
usehas an optimization where in some cases it can suspend the work loop during the render phase until the data has resolved, rather than unwind the stack and lose context. However, the current implementation is not compatible with sibling prerendering. So I've temporarily disabled it until the sibling prerendering has been refactored. We will add it back in a later step.