Skip to content

Commit 206188f

Browse files
committed
Fix bug that wasn't picking up the right stack at suspended boundaries
This makes it more explicit which stack we pass in to be retained by the task.
1 parent 8f28fba commit 206188f

File tree

2 files changed

+118
-2
lines changed

2 files changed

+118
-2
lines changed

packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js

Lines changed: 104 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1058,6 +1058,110 @@ describe('ReactDOMFizzServer', () => {
10581058
);
10591059
});
10601060

1061+
function normalizeCodeLocInfo(str) {
1062+
return (
1063+
str &&
1064+
str.replace(/\n +(?:at|in) ([\S]+)[^\n]*/g, function(m, name) {
1065+
return '\n in ' + name + ' (at **)';
1066+
})
1067+
);
1068+
}
1069+
1070+
// @gate experimental
1071+
it('should include a component stack across suspended boundaries', async () => {
1072+
function B() {
1073+
const children = [readText('Hello'), readText('World')];
1074+
// Intentionally trigger a key warning here.
1075+
return (
1076+
<div>
1077+
{children.map(t => (
1078+
<span>{t}</span>
1079+
))}
1080+
</div>
1081+
);
1082+
}
1083+
function C() {
1084+
return (
1085+
<inCorrectTag>
1086+
<Text text="Loading" />
1087+
</inCorrectTag>
1088+
);
1089+
}
1090+
function A() {
1091+
return (
1092+
<div>
1093+
<Suspense fallback={<C />}>
1094+
<B />
1095+
</Suspense>
1096+
</div>
1097+
);
1098+
}
1099+
1100+
// We can't use the toErrorDev helper here because this is an async act.
1101+
const originalConsoleError = console.error;
1102+
const mockError = jest.fn();
1103+
console.error = (...args) => {
1104+
mockError(...args.map(normalizeCodeLocInfo));
1105+
};
1106+
1107+
try {
1108+
await act(async () => {
1109+
const {startWriting} = ReactDOMFizzServer.pipeToNodeWritable(
1110+
<A />,
1111+
writable,
1112+
);
1113+
startWriting();
1114+
});
1115+
1116+
expect(getVisibleChildren(container)).toEqual(
1117+
<div>
1118+
<incorrecttag>Loading</incorrecttag>
1119+
</div>,
1120+
);
1121+
1122+
expect(mockError).toHaveBeenCalledWith(
1123+
'Warning: <%s /> is using incorrect casing. Use PascalCase for React components, or lowercase for HTML elements.%s',
1124+
'inCorrectTag',
1125+
'\n' +
1126+
' in inCorrectTag (at **)\n' +
1127+
' in C (at **)\n' +
1128+
' in Suspense (at **)\n' +
1129+
' in div (at **)\n' +
1130+
' in A (at **)',
1131+
);
1132+
mockError.mockClear();
1133+
1134+
await act(async () => {
1135+
resolveText('Hello');
1136+
resolveText('World');
1137+
});
1138+
1139+
expect(mockError).toHaveBeenCalledWith(
1140+
'Warning: Each child in a list should have a unique "key" prop.%s%s' +
1141+
' See https://reactjs.org/link/warning-keys for more information.%s',
1142+
'\n\nCheck the top-level render call using <div>.',
1143+
'',
1144+
'\n' +
1145+
' in span (at **)\n' +
1146+
' in B (at **)\n' +
1147+
' in Suspense (at **)\n' +
1148+
' in div (at **)\n' +
1149+
' in A (at **)',
1150+
);
1151+
1152+
expect(getVisibleChildren(container)).toEqual(
1153+
<div>
1154+
<div>
1155+
<span>Hello</span>
1156+
<span>World</span>
1157+
</div>
1158+
</div>,
1159+
);
1160+
} finally {
1161+
console.error = originalConsoleError;
1162+
}
1163+
});
1164+
10611165
// @gate experimental
10621166
it('should can suspend in a class component with legacy context', async () => {
10631167
class TestProvider extends React.Component {

packages/react-server/src/ReactFizzServer.js

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -314,7 +314,7 @@ function createTask(
314314
assignID,
315315
}: any);
316316
if (__DEV__) {
317-
task.componentStack = currentTaskInDEV ? task.componentStack : null;
317+
task.componentStack = null;
318318
}
319319
abortSet.add(task);
320320
return task;
@@ -465,6 +465,7 @@ function renderSuspenseBoundary(
465465
// This must have been the last segment we were waiting on. This boundary is now complete.
466466
// Therefore we won't need the fallback. We early return so that we don't have to create
467467
// the fallback.
468+
popComponentStackInDEV(task);
468469
return;
469470
}
470471
} catch (error) {
@@ -477,7 +478,6 @@ function renderSuspenseBoundary(
477478
} finally {
478479
task.blockedBoundary = parentBoundary;
479480
task.blockedSegment = parentSegment;
480-
popComponentStackInDEV(task);
481481
}
482482

483483
// This injects an extra segment just to contain an empty tag with an ID.
@@ -505,9 +505,14 @@ function renderSuspenseBoundary(
505505
task.context,
506506
null,
507507
);
508+
if (__DEV__) {
509+
suspendedFallbackTask.componentStack = task.componentStack;
510+
}
508511
// TODO: This should be queued at a separate lower priority queue so that we only work
509512
// on preparing fallbacks if we don't have any more main content to task on.
510513
request.pingedTasks.push(suspendedFallbackTask);
514+
515+
popComponentStackInDEV(task);
511516
}
512517

513518
function renderHostElement(
@@ -1233,6 +1238,13 @@ function spawnNewSuspendedTask(
12331238
task.context,
12341239
task.assignID,
12351240
);
1241+
if (__DEV__) {
1242+
if (task.componentStack !== null) {
1243+
// We pop one task off the stack because the node that suspended will be tried again,
1244+
// which will add it back onto the stack.
1245+
newTask.componentStack = task.componentStack.parent;
1246+
}
1247+
}
12361248
// We've delegated the assignment.
12371249
task.assignID = null;
12381250
const ping = newTask.ping;

0 commit comments

Comments
 (0)