Skip to content
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

[Fizz] Mark boundary as client rendered even if aborting fallback #21294

Merged
merged 1 commit into from
Apr 15, 2021

Conversation

sebmarkbage
Copy link
Collaborator

@sebmarkbage sebmarkbage commented Apr 15, 2021

The fix in 5781269 was slightly wrong. This surfaced in a legacy test. /~https://github.com/facebook/react/blob/master/packages/react-dom/src/__tests__/ReactDOMServerPartialHydration-test.internal.js

If a task blocking a boundary is aborted, and that boundary's fallback has pending tasks, it doesn't mean that the parent will be client rendered. Because the boundary's fallback's pending tasks might themselves be inside other boundaries and not needed to complete. So we must still mark it as client rendered in that case.

We didn't need to be this conservative because the real fix was moving the allPendingTasks decrementor. As a safety for new user space calls, I made sure to move the parentFlushed check before the recursive call.

@sebmarkbage sebmarkbage requested a review from gaearon April 15, 2021 23:09
@facebook-github-bot facebook-github-bot added CLA Signed React Core Team Opened by a member of the React Core Team labels Apr 15, 2021
@sebmarkbage sebmarkbage merged commit cc4b431 into facebook:master Apr 15, 2021
@sizebot
Copy link

sizebot commented Apr 16, 2021

Comparing: f7cdc89...b7201cc

Critical size changes

Includes critical production bundles, as well as any change greater than 2%:

Name +/- Base Current +/- gzip Base gzip Current gzip
oss-stable/react-dom/cjs/react-dom.production.min.js = 122.72 kB 122.72 kB = 39.39 kB 39.39 kB
oss-experimental/react-dom/cjs/react-dom.production.min.js = 129.30 kB 129.30 kB = 41.48 kB 41.47 kB
facebook-www/ReactDOM-prod.classic.js = 412.22 kB 412.22 kB = 76.24 kB 76.23 kB
facebook-www/ReactDOM-prod.modern.js = 400.29 kB 400.29 kB = 74.32 kB 74.32 kB
facebook-www/ReactDOMForked-prod.classic.js = 412.22 kB 412.22 kB = 76.24 kB 76.24 kB

Significant size changes

Includes any change greater than 0.2%:

Expand to show
Name +/- Base Current +/- gzip Base gzip Current gzip
oss-experimental/react-server/cjs/react-server.development.js = 110.64 kB 110.42 kB = 27.57 kB 27.53 kB
oss-stable/react-server/cjs/react-server.development.js = 110.08 kB 109.86 kB = 27.42 kB 27.38 kB

Generated by 🚫 dangerJS against b7201cc

facebook-github-bot pushed a commit to facebook/react-native that referenced this pull request Apr 21, 2021
Summary:
This sync includes the following changes:
- **[bd7f4a013](facebook/react@bd7f4a013 )**: Fix sloppy factoring in `performSyncWorkOnRoot` ([#21246](facebook/react#21246)) //<Andrew Clark>//
- **[78120032d](facebook/react@78120032d )**: Remove `flushDiscreteUpdates` from end of event ([#21223](facebook/react#21223)) //<Andrew Clark>//
- **[a3a7adb83](facebook/react@a3a7adb83 )**: Turn off enableSyncDefaultUpdates in test renderer ([#21319](facebook/react#21319)) //<Ricky>//
- **[cdb6b4c55](facebook/react@cdb6b4c55 )**: Only hide outermost host nodes when Offscreen is hidden ([#21250](facebook/react#21250)) //<Brian Vaughn>//
- **[b9c6a2b30](facebook/react@b9c6a2b30 )**: Remove LayoutStatic check from commit phase ([#21249](facebook/react#21249)) //<Brian Vaughn>//
- **[af1a4cbf7](facebook/react@af1a4cbf7 )**: Revert expiration for retry lanes ([#21300](facebook/react#21300)) //<Andrew Clark>//
- **[cc4b431da](facebook/react@cc4b431da )**: Mark boundary as client rendered even if aborting fallback ([#21294](facebook/react#21294)) //<Sebastian Markbåge>//

Changelog:
[General][Changed] - React Native sync for revisions f7cdc89...bd7f4a0

jest_e2e[run_all_tests]

Reviewed By: JoshuaGross

Differential Revision: D27909257

fbshipit-source-id: 36ec4cf1de9df109f1fe1542031df10a693baae0
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed React Core Team Opened by a member of the React Core Team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants