@samitouri / QOS-React-2 / commits / ebf7318e87

Hide/unhide the content of dehydrated suspense boundaries if they resuspend (#32900)

Found this bug while working on Activity. There's a weird edge case when a dehydrated Suspense boundary is a direct child of another Suspense boundary which is hydrated but then it resuspends without forcing the inner one to hydrate/delete. It used to just leave that in place because hiding/unhiding didn't deal with dehydrated fragments. Not sure this is really worth fixing.

Sebastian Markbåge committed Apr 22, 2025 at 19:29 UTC ebf7318e87cf2e10b6bd9a6bb0ad8bf6f6186f80
6 files changed +183
packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js
+61
@@ -1127,6 +1127,61 @@ export function clearSuspenseBoundaryFromContainer(
1127 retryIfBlockedOn(container);
1128 }
1129
1130 +function hideOrUnhideSuspenseBoundary(
1131 + suspenseInstance: SuspenseInstance,
1132 + isHidden: boolean,
1133 +) {
1134 + let node: Node = suspenseInstance;
1135 + // Unhide all nodes within this suspense boundary.
1136 + let depth = 0;
1137 + do {
1138 + const nextNode = node.nextSibling;
1139 + if (node.nodeType === ELEMENT_NODE) {
1140 + const instance = ((node: any): HTMLElement & {_stashedDisplay?: string});
1141 + if (isHidden) {
1142 + instance._stashedDisplay = instance.style.display;
1143 + instance.style.display = 'none';
1144 + } else {
1145 + instance.style.display = instance._stashedDisplay || '';
1146 + if (instance.getAttribute('style') === '') {
1147 + instance.removeAttribute('style');
1148 + }
1149 + }
1150 + } else if (node.nodeType === TEXT_NODE) {
1151 + const textNode = ((node: any): Text & {_stashedText?: string});
1152 + if (isHidden) {
1153 + textNode._stashedText = textNode.nodeValue;
1154 + textNode.nodeValue = '';
1155 + } else {
1156 + textNode.nodeValue = textNode._stashedText || '';
1157 + }
1158 + }
1159 + if (nextNode && nextNode.nodeType === COMMENT_NODE) {
1160 + const data = ((nextNode: any).data: string);
1161 + if (data === SUSPENSE_END_DATA) {
1162 + if (depth === 0) {
1163 + return;
1164 + } else {
1165 + depth--;
1166 + }
1167 + } else if (
1168 + data === SUSPENSE_START_DATA ||
1169 + data === SUSPENSE_PENDING_START_DATA ||
1170 + data === SUSPENSE_FALLBACK_START_DATA
1171 + ) {
1172 + depth++;
1173 + }
1174 + // TODO: Should we hide preamble contribution in this case?
1175 + }
1176 + // $FlowFixMe[incompatible-type] we bail out when we get a null
1177 + node = nextNode;
1178 + } while (node);
1179 +}
1180 +
1181 +export function hideSuspenseBoundary(suspenseInstance: SuspenseInstance): void {
1182 + hideOrUnhideSuspenseBoundary(suspenseInstance, true);
1183 +}
1184 +
1185 export function hideInstance(instance: Instance): void {
1186 // TODO: Does this work for all element types? What about MathML? Should we
1187 // pass host context to this method?
@@ -1144,6 +1199,12 @@ export function hideTextInstance(textInstance: TextInstance): void {
1199 textInstance.nodeValue = '';
1200 }
1201
1202 +export function unhideSuspenseBoundary(
1203 + suspenseInstance: SuspenseInstance,
1204 +): void {
1205 + hideOrUnhideSuspenseBoundary(suspenseInstance, false);
1206 +}
1207 +
1208 export function unhideInstance(instance: Instance, props: Props): void {
1209 instance = ((instance: any): HTMLElement);
1210 const styleProp = props[STYLE];
packages/react-dom/src/__tests__/ReactDOMServerPartialHydration-test.internal.js
+90
@@ -3986,4 +3986,94 @@ describe('ReactDOMServerPartialHydration', () => {
3986 "onRecoverableError: Hydration failed because the server rendered text didn't match the client.",
3987 ]);
3988 });
3989 +
3990 + it('hides a dehydrated suspense boundary if the parent resuspends', async () => {
3991 + let suspend = false;
3992 + let resolve;
3993 + const promise = new Promise(resolvePromise => (resolve = resolvePromise));
3994 + const ref = React.createRef();
3995 +
3996 + function Child({text}) {
3997 + if (suspend) {
3998 + throw promise;
3999 + } else {
4000 + return text;
4001 + }
4002 + }
4003 +
4004 + function Sibling({resuspend}) {
4005 + if (suspend && resuspend) {
4006 + throw promise;
4007 + } else {
4008 + return null;
4009 + }
4010 + }
4011 +
4012 + function Component({text}) {
4013 + return (
4014 + <Suspense>
4015 + <Child text={text} />
4016 + <span ref={ref}>World</span>
4017 + </Suspense>
4018 + );
4019 + }
4020 +
4021 + function App({text, resuspend}) {
4022 + const memoized = React.useMemo(() => <Component text={text} />, [text]);
4023 + return (
4024 + <div>
4025 + <Suspense fallback="Loading...">
4026 + {memoized}
4027 + <Sibling resuspend={resuspend} />
4028 + </Suspense>
4029 + </div>
4030 + );
4031 + }
4032 +
4033 + suspend = false;
4034 + const finalHTML = ReactDOMServer.renderToString(<App text="Hello" />);
4035 + const container = document.createElement('div');
4036 + container.innerHTML = finalHTML;
4037 +
4038 + // On the client we don't have all data yet but we want to start
4039 + // hydrating anyway.
4040 + suspend = true;
4041 + const root = ReactDOMClient.hydrateRoot(container, <App text="Hello" />, {
4042 + onRecoverableError(error) {
4043 + Scheduler.log('onRecoverableError: ' + normalizeError(error.message));
4044 + if (error.cause) {
4045 + Scheduler.log('Cause: ' + normalizeError(error.cause.message));
4046 + }
4047 + },
4048 + });
4049 + await waitForAll([]);
4050 +
4051 + expect(ref.current).toBe(null); // Still dehydrated
4052 + const span = container.getElementsByTagName('span')[0];
4053 + const textNode = span.previousSibling;
4054 + expect(textNode.nodeValue).toBe('Hello');
4055 + expect(span.textContent).toBe('World');
4056 +
4057 + // Render an update, that resuspends the parent boundary.
4058 + // Flushing now now hide the text content.
4059 + await act(() => {
4060 + root.render(<App text="Hello" resuspend={true} />);
4061 + });
4062 +
4063 + expect(ref.current).toBe(null);
4064 + expect(span.style.display).toBe('none');
4065 + expect(textNode.nodeValue).toBe('');
4066 +
4067 + // Unsuspending shows the content.
4068 + await act(async () => {
4069 + suspend = false;
4070 + resolve();
4071 + await promise;
4072 + });
4073 +
4074 + expect(textNode.nodeValue).toBe('Hello');
4075 + expect(span.textContent).toBe('World');
4076 + expect(span.style.display).toBe('');
4077 + expect(ref.current).toBe(span);
4078 + });
4079 });
packages/react-reconciler/src/ReactFiberCommitHostEffects.js
+23
@@ -41,8 +41,10 @@ import {
41 insertBefore,
42 insertInContainerBefore,
43 replaceContainerChildren,
44 + hideSuspenseBoundary,
45 hideInstance,
46 hideTextInstance,
47 + unhideSuspenseBoundary,
48 unhideInstance,
49 unhideTextInstance,
50 commitHydratedContainer,
@@ -152,6 +154,27 @@ export function commitHostResetTextContent(finishedWork: Fiber) {
154 }
155 }
156
157 +export function commitShowHideSuspenseBoundary(node: Fiber, isHidden: boolean) {
158 + try {
159 + const instance = node.stateNode;
160 + if (isHidden) {
161 + if (__DEV__) {
162 + runWithFiberInDEV(node, hideSuspenseBoundary, instance);
163 + } else {
164 + hideSuspenseBoundary(instance);
165 + }
166 + } else {
167 + if (__DEV__) {
168 + runWithFiberInDEV(node, unhideSuspenseBoundary, node.stateNode);
169 + } else {
170 + unhideSuspenseBoundary(node.stateNode);
171 + }
172 + }
173 + } catch (error) {
174 + captureCommitPhaseError(node, node.return, error);
175 + }
176 +}
177 +
178 export function commitShowHideHostInstance(node: Fiber, isHidden: boolean) {
179 try {
180 const instance = node.stateNode;
packages/react-reconciler/src/ReactFiberCommitWork.js
+5
@@ -227,6 +227,7 @@ import {
227 commitHostUpdate,
228 commitHostTextUpdate,
229 commitHostResetTextContent,
230 + commitShowHideSuspenseBoundary,
231 commitShowHideHostInstance,
232 commitShowHideHostTextInstance,
233 commitHostPlacement,
@@ -1158,6 +1159,10 @@ function hideOrUnhideAllChildren(finishedWork: Fiber, isHidden: boolean) {
1159 if (hostSubtreeRoot === null) {
1160 commitShowHideHostTextInstance(node, isHidden);
1161 }
1162 + } else if (node.tag === DehydratedFragment) {
1163 + if (hostSubtreeRoot === null) {
1164 + commitShowHideSuspenseBoundary(node, isHidden);
1165 + }
1166 } else if (
1167 (node.tag === OffscreenComponent ||
1168 node.tag === LegacyHiddenComponent) &&
packages/react-reconciler/src/ReactFiberConfigWithNoHydration.js
+2
@@ -44,6 +44,8 @@ export const commitHydratedContainer = shim;
44 export const commitHydratedSuspenseInstance = shim;
45 export const clearSuspenseBoundary = shim;
46 export const clearSuspenseBoundaryFromContainer = shim;
47 +export const hideSuspenseBoundary = shim;
48 +export const unhideSuspenseBoundary = shim;
49 export const shouldDeleteUnhydratedTailInstances = shim;
50 export const diffHydratedPropsForDevWarnings = shim;
51 export const diffHydratedTextForDevWarnings = shim;
packages/react-reconciler/src/forks/ReactFiberConfig.custom.js
+2
@@ -220,6 +220,8 @@ export const commitHydratedSuspenseInstance =
220 export const clearSuspenseBoundary = $$$config.clearSuspenseBoundary;
221 export const clearSuspenseBoundaryFromContainer =
222 $$$config.clearSuspenseBoundaryFromContainer;
223 +export const hideSuspenseBoundary = $$$config.hideSuspenseBoundary;
224 +export const unhideSuspenseBoundary = $$$config.unhideSuspenseBoundary;
225 export const shouldDeleteUnhydratedTailInstances =
226 $$$config.shouldDeleteUnhydratedTailInstances;
227 export const diffHydratedPropsForDevWarnings =