Skip to content

Commit c2c6ea1

Browse files
rickhanloniirickhanlonii
authored
Capture suspense boundaries with undefined fallbacks (#21854)
1 parent bfa50f8 commit c2c6ea1

14 files changed

+452
-129
lines changed

‎packages/react-dom/src/__tests__/ReactDOMServerPartialHydration-test.internal.js

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -714,7 +714,7 @@ describe('ReactDOMServerPartialHydration', () => {
714714
expect(container.textContent).toBe('Hi');
715715
});
716716

717-
it('shows the fallback of the outer if fallback is missing', async () => {
717+
it('treats missing fallback the same as if it was defined', async () => {
718718
// This is the same exact test as above but with a nested Suspense without a fallback.
719719
// This should be a noop.
720720
let suspend = false;
@@ -759,7 +759,8 @@ describe('ReactDOMServerPartialHydration', () => {
759759
Scheduler.unstable_flushAll();
760760
jest.runAllTimers();
761761

762-
expect(ref.current).toBe(null);
762+
const span = container.getElementsByTagName('span')[0];
763+
expect(ref.current).toBe(span);
763764

764765
// Render an update, but leave it still suspended.
765766
root.render(<App text="Hi" className="hi" />);
@@ -768,9 +769,9 @@ describe('ReactDOMServerPartialHydration', () => {
768769
Scheduler.unstable_flushAll();
769770
jest.runAllTimers();
770771

771-
expect(container.getElementsByTagName('span').length).toBe(0);
772-
expect(ref.current).toBe(null);
773-
expect(container.textContent).toBe('Loading...');
772+
expect(container.getElementsByTagName('span').length).toBe(1);
773+
expect(ref.current).toBe(span);
774+
expect(container.textContent).toBe('');
774775

775776
// Unsuspending shows the content.
776777
suspend = false;
@@ -780,7 +781,6 @@ describe('ReactDOMServerPartialHydration', () => {
780781
Scheduler.unstable_flushAll();
781782
jest.runAllTimers();
782783

783-
const span = container.getElementsByTagName('span')[0];
784784
expect(span.textContent).toBe('Hi');
785785
expect(span.className).toBe('hi');
786786
expect(ref.current).toBe(span);

‎packages/react-dom/src/__tests__/ReactServerRenderingHydration-test.js

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -471,10 +471,12 @@ describe('ReactDOMServerHydration', () => {
471471
element,
472472
);
473473

474-
// Because this didn't have a fallback, it was hydrated as if it's
475-
// not a Suspense boundary.
476-
expect(ref.current).toBe(div);
477-
expect(element.innerHTML).toBe('<div>Hello World</div>');
474+
// The content should've been client rendered.
475+
expect(ref.current).not.toBe(div);
476+
// Unfortunately, since we don't delete the tail at the root, a duplicate will remain.
477+
expect(element.innerHTML).toBe(
478+
'<div>Hello World</div><div>Hello World</div>',
479+
);
478480
});
479481

480482
// regression test for https://github.com/facebook/react/issues/17170

‎packages/react-dom/src/server/ReactPartialRenderer.js

Lines changed: 0 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1123,25 +1123,6 @@ class ReactDOMServerRenderer {
11231123
case REACT_SUSPENSE_TYPE: {
11241124
if (enableSuspenseServerRenderer) {
11251125
const fallback = ((nextChild: any): ReactElement).props.fallback;
1126-
if (fallback === undefined) {
1127-
// If there is no fallback, then this just behaves as a fragment.
1128-
const nextChildren = toArray(
1129-
((nextChild: any): ReactElement).props.children,
1130-
);
1131-
const frame: Frame = {
1132-
type: null,
1133-
domNamespace: parentNamespace,
1134-
children: nextChildren,
1135-
childIndex: 0,
1136-
context: context,
1137-
footer: '',
1138-
};
1139-
if (__DEV__) {
1140-
((frame: any): FrameDev).debugElementStack = [];
1141-
}
1142-
this.stack.push(frame);
1143-
return '';
1144-
}
11451126
const fallbackChildren = toArray(fallback);
11461127
const nextChildren = toArray(
11471128
((nextChild: any): ReactElement).props.children,

‎packages/react-reconciler/src/ReactFiberBeginWork.new.js

Lines changed: 14 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -1867,12 +1867,8 @@ function updateSuspenseComponent(current, workInProgress, renderLanes) {
18671867
// This is a new mount or this boundary is already showing a fallback state.
18681868
// Mark this subtree context as having at least one invisible parent that could
18691869
// handle the fallback state.
1870-
// Boundaries without fallbacks or should be avoided are not considered since
1871-
// they cannot handle preferred fallback states.
1872-
if (
1873-
nextProps.fallback !== undefined &&
1874-
nextProps.unstable_avoidThisFallback !== true
1875-
) {
1870+
// Avoided boundaries are not considered since they cannot handle preferred fallback states.
1871+
if (nextProps.unstable_avoidThisFallback !== true) {
18761872
suspenseContext = addSubtreeSuspenseContext(
18771873
suspenseContext,
18781874
InvisibleParentSuspenseContext,
@@ -1910,22 +1906,18 @@ function updateSuspenseComponent(current, workInProgress, renderLanes) {
19101906
if (current === null) {
19111907
// Initial mount
19121908
// If we're currently hydrating, try to hydrate this boundary.
1913-
// But only if this has a fallback.
1914-
if (nextProps.fallback !== undefined) {
1915-
tryToClaimNextHydratableInstance(workInProgress);
1916-
// This could've been a dehydrated suspense component.
1917-
if (enableSuspenseServerRenderer) {
1918-
const suspenseState: null | SuspenseState =
1919-
workInProgress.memoizedState;
1920-
if (suspenseState !== null) {
1921-
const dehydrated = suspenseState.dehydrated;
1922-
if (dehydrated !== null) {
1923-
return mountDehydratedSuspenseComponent(
1924-
workInProgress,
1925-
dehydrated,
1926-
renderLanes,
1927-
);
1928-
}
1909+
tryToClaimNextHydratableInstance(workInProgress);
1910+
// This could've been a dehydrated suspense component.
1911+
if (enableSuspenseServerRenderer) {
1912+
const suspenseState: null | SuspenseState = workInProgress.memoizedState;
1913+
if (suspenseState !== null) {
1914+
const dehydrated = suspenseState.dehydrated;
1915+
if (dehydrated !== null) {
1916+
return mountDehydratedSuspenseComponent(
1917+
workInProgress,
1918+
dehydrated,
1919+
renderLanes,
1920+
);
19291921
}
19301922
}
19311923
}

‎packages/react-reconciler/src/ReactFiberBeginWork.old.js

Lines changed: 14 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -1867,12 +1867,8 @@ function updateSuspenseComponent(current, workInProgress, renderLanes) {
18671867
// This is a new mount or this boundary is already showing a fallback state.
18681868
// Mark this subtree context as having at least one invisible parent that could
18691869
// handle the fallback state.
1870-
// Boundaries without fallbacks or should be avoided are not considered since
1871-
// they cannot handle preferred fallback states.
1872-
if (
1873-
nextProps.fallback !== undefined &&
1874-
nextProps.unstable_avoidThisFallback !== true
1875-
) {
1870+
// Avoided boundaries are not considered since they cannot handle preferred fallback states.
1871+
if (nextProps.unstable_avoidThisFallback !== true) {
18761872
suspenseContext = addSubtreeSuspenseContext(
18771873
suspenseContext,
18781874
InvisibleParentSuspenseContext,
@@ -1910,22 +1906,18 @@ function updateSuspenseComponent(current, workInProgress, renderLanes) {
19101906
if (current === null) {
19111907
// Initial mount
19121908
// If we're currently hydrating, try to hydrate this boundary.
1913-
// But only if this has a fallback.
1914-
if (nextProps.fallback !== undefined) {
1915-
tryToClaimNextHydratableInstance(workInProgress);
1916-
// This could've been a dehydrated suspense component.
1917-
if (enableSuspenseServerRenderer) {
1918-
const suspenseState: null | SuspenseState =
1919-
workInProgress.memoizedState;
1920-
if (suspenseState !== null) {
1921-
const dehydrated = suspenseState.dehydrated;
1922-
if (dehydrated !== null) {
1923-
return mountDehydratedSuspenseComponent(
1924-
workInProgress,
1925-
dehydrated,
1926-
renderLanes,
1927-
);
1928-
}
1909+
tryToClaimNextHydratableInstance(workInProgress);
1910+
// This could've been a dehydrated suspense component.
1911+
if (enableSuspenseServerRenderer) {
1912+
const suspenseState: null | SuspenseState = workInProgress.memoizedState;
1913+
if (suspenseState !== null) {
1914+
const dehydrated = suspenseState.dehydrated;
1915+
if (dehydrated !== null) {
1916+
return mountDehydratedSuspenseComponent(
1917+
workInProgress,
1918+
dehydrated,
1919+
renderLanes,
1920+
);
19291921
}
19301922
}
19311923
}

‎packages/react-reconciler/src/ReactFiberCompleteWork.new.js

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1045,9 +1045,7 @@ function completeWork(
10451045
const nextDidTimeout = nextState !== null;
10461046
let prevDidTimeout = false;
10471047
if (current === null) {
1048-
if (workInProgress.memoizedProps.fallback !== undefined) {
1049-
popHydrationState(workInProgress);
1050-
}
1048+
popHydrationState(workInProgress);
10511049
} else {
10521050
const prevState: null | SuspenseState = current.memoizedState;
10531051
prevDidTimeout = prevState !== null;

‎packages/react-reconciler/src/ReactFiberCompleteWork.old.js

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1045,9 +1045,7 @@ function completeWork(
10451045
const nextDidTimeout = nextState !== null;
10461046
let prevDidTimeout = false;
10471047
if (current === null) {
1048-
if (workInProgress.memoizedProps.fallback !== undefined) {
1049-
popHydrationState(workInProgress);
1050-
}
1048+
popHydrationState(workInProgress);
10511049
} else {
10521050
const prevState: null | SuspenseState = current.memoizedState;
10531051
prevDidTimeout = prevState !== null;

‎packages/react-reconciler/src/ReactFiberSuspenseComponent.new.js

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -77,10 +77,6 @@ export function shouldCaptureSuspense(
7777
return false;
7878
}
7979
const props = workInProgress.memoizedProps;
80-
// In order to capture, the Suspense component must have a fallback prop.
81-
if (props.fallback === undefined) {
82-
return false;
83-
}
8480
// Regular boundaries always capture.
8581
if (props.unstable_avoidThisFallback !== true) {
8682
return true;

‎packages/react-reconciler/src/ReactFiberSuspenseComponent.old.js

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -77,10 +77,6 @@ export function shouldCaptureSuspense(
7777
return false;
7878
}
7979
const props = workInProgress.memoizedProps;
80-
// In order to capture, the Suspense component must have a fallback prop.
81-
if (props.fallback === undefined) {
82-
return false;
83-
}
8480
// Regular boundaries always capture.
8581
if (props.unstable_avoidThisFallback !== true) {
8682
return true;

‎packages/react-reconciler/src/__tests__/ReactHooks-test.internal.js

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,6 @@ let Scheduler;
1919
let ReactDOMServer;
2020
let act;
2121

22-
// Additional tests can be found in ReactHooksWithNoopRenderer. Plan is to
23-
// gradually migrate those to this file.
2422
describe('ReactHooks', () => {
2523
beforeEach(() => {
2624
jest.resetModules();

‎packages/react-reconciler/src/__tests__/ReactSuspense-test.internal.js

Lines changed: 4 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,6 @@ let act;
99
let TextResource;
1010
let textResourceShouldFail;
1111

12-
// Additional tests can be found in ReactSuspenseWithNoopRenderer. Plan is
13-
// to gradually migrate those to this file.
1412
describe('ReactSuspense', () => {
1513
beforeEach(() => {
1614
jest.resetModules();
@@ -391,44 +389,10 @@ describe('ReactSuspense', () => {
391389
expect(root).toMatchRenderedOutput('Hi');
392390
});
393391

394-
it('only captures if `fallback` is defined', () => {
395-
const root = ReactTestRenderer.create(
396-
<Suspense fallback={<Text text="Loading..." />}>
397-
<Suspense>
398-
<AsyncText text="Hi" ms={5000} />
399-
</Suspense>
400-
</Suspense>,
401-
{
402-
unstable_isConcurrent: true,
403-
},
404-
);
405-
406-
expect(Scheduler).toFlushAndYield([
407-
'Suspend! [Hi]',
408-
// The outer fallback should be rendered, because the inner one does not
409-
// have a `fallback` prop
410-
'Loading...',
411-
]);
412-
jest.advanceTimersByTime(1000);
413-
expect(Scheduler).toHaveYielded([]);
414-
expect(Scheduler).toFlushAndYield([]);
415-
expect(root).toMatchRenderedOutput('Loading...');
416-
417-
jest.advanceTimersByTime(5000);
418-
expect(Scheduler).toHaveYielded(['Promise resolved [Hi]']);
419-
expect(Scheduler).toFlushAndYield(['Hi']);
420-
expect(root).toMatchRenderedOutput('Hi');
421-
});
422-
423-
it('throws if tree suspends and none of the Suspense ancestors have a fallback', () => {
424-
ReactTestRenderer.create(
425-
<Suspense>
426-
<AsyncText text="Hi" ms={1000} />
427-
</Suspense>,
428-
{
429-
unstable_isConcurrent: true,
430-
},
431-
);
392+
it('throws if tree suspends and none of the Suspense ancestors have a boundary', () => {
393+
ReactTestRenderer.create(<AsyncText text="Hi" ms={1000} />, {
394+
unstable_isConcurrent: true,
395+
});
432396

433397
expect(Scheduler).toFlushAndThrow(
434398
'AsyncText suspended while rendering, but no fallback UI was specified.',

0 commit comments

Comments
 (0)