Skip to content

Commit

Permalink
Fix bug that wasn't picking up the right stack at suspended boundaries
Browse files Browse the repository at this point in the history
This makes it more explicit which stack we pass in to be retained by the
task.
  • Loading branch information
sebmarkbage committed Jun 3, 2021
1 parent 8f28fba commit 3d2a1ab
Show file tree
Hide file tree
Showing 2 changed files with 126 additions and 2 deletions.
112 changes: 112 additions & 0 deletions packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -1058,6 +1058,118 @@ describe('ReactDOMFizzServer', () => {
);
});

function normalizeCodeLocInfo(str) {
return (
str &&
str.replace(/\n +(?:at|in) ([\S]+)[^\n]*/g, function(m, name) {
return '\n in ' + name + ' (at **)';
})
);
}

// @gate experimental
it('should include a component stack across suspended boundaries', async () => {
function B() {
const children = [readText('Hello'), readText('World')];
// Intentionally trigger a key warning here.
return (
<div>
{children.map(t => (
<span>{t}</span>
))}
</div>
);
}
function C() {
return (
<inCorrectTag>
<Text text="Loading" />
</inCorrectTag>
);
}
function A() {
return (
<div>
<Suspense fallback={<C />}>
<B />
</Suspense>
</div>
);
}

// We can't use the toErrorDev helper here because this is an async act.
const originalConsoleError = console.error;
const mockError = jest.fn();
console.error = (...args) => {
mockError(...args.map(normalizeCodeLocInfo));
};

try {
await act(async () => {
const {startWriting} = ReactDOMFizzServer.pipeToNodeWritable(
<A />,
writable,
);
startWriting();
});

expect(getVisibleChildren(container)).toEqual(
<div>
<incorrecttag>Loading</incorrecttag>
</div>,
);

if (__DEV__) {
expect(mockError).toHaveBeenCalledWith(
'Warning: <%s /> is using incorrect casing. Use PascalCase for React components, or lowercase for HTML elements.%s',
'inCorrectTag',
'\n' +
' in inCorrectTag (at **)\n' +
' in C (at **)\n' +
' in Suspense (at **)\n' +
' in div (at **)\n' +
' in A (at **)',
);
mockError.mockClear();
} else {
expect(mockError).not.toHaveBeenCalled();
}

await act(async () => {
resolveText('Hello');
resolveText('World');
});

if (__DEV__) {
expect(mockError).toHaveBeenCalledWith(
'Warning: Each child in a list should have a unique "key" prop.%s%s' +
' See https://reactjs.org/link/warning-keys for more information.%s',
'\n\nCheck the top-level render call using <div>.',
'',
'\n' +
' in span (at **)\n' +
' in B (at **)\n' +
' in Suspense (at **)\n' +
' in div (at **)\n' +
' in A (at **)',
);
} else {
expect(mockError).not.toHaveBeenCalled();
}

expect(getVisibleChildren(container)).toEqual(
<div>
<div>
<span>Hello</span>
<span>World</span>
</div>
</div>,
);
} finally {
console.error = originalConsoleError;
}
});

// @gate experimental
it('should can suspend in a class component with legacy context', async () => {
class TestProvider extends React.Component {
Expand Down
16 changes: 14 additions & 2 deletions packages/react-server/src/ReactFizzServer.js
Original file line number Diff line number Diff line change
Expand Up @@ -314,7 +314,7 @@ function createTask(
assignID,
}: any);
if (__DEV__) {
task.componentStack = currentTaskInDEV ? task.componentStack : null;
task.componentStack = null;
}
abortSet.add(task);
return task;
Expand Down Expand Up @@ -465,6 +465,7 @@ function renderSuspenseBoundary(
// This must have been the last segment we were waiting on. This boundary is now complete.
// Therefore we won't need the fallback. We early return so that we don't have to create
// the fallback.
popComponentStackInDEV(task);
return;
}
} catch (error) {
Expand All @@ -477,7 +478,6 @@ function renderSuspenseBoundary(
} finally {
task.blockedBoundary = parentBoundary;
task.blockedSegment = parentSegment;
popComponentStackInDEV(task);
}

// This injects an extra segment just to contain an empty tag with an ID.
Expand Down Expand Up @@ -505,9 +505,14 @@ function renderSuspenseBoundary(
task.context,
null,
);
if (__DEV__) {
suspendedFallbackTask.componentStack = task.componentStack;
}
// TODO: This should be queued at a separate lower priority queue so that we only work
// on preparing fallbacks if we don't have any more main content to task on.
request.pingedTasks.push(suspendedFallbackTask);

popComponentStackInDEV(task);
}

function renderHostElement(
Expand Down Expand Up @@ -1233,6 +1238,13 @@ function spawnNewSuspendedTask(
task.context,
task.assignID,
);
if (__DEV__) {
if (task.componentStack !== null) {
// We pop one task off the stack because the node that suspended will be tried again,
// which will add it back onto the stack.
newTask.componentStack = task.componentStack.parent;
}
}
// We've delegated the assignment.
task.assignID = null;
const ping = newTask.ping;
Expand Down

0 comments on commit 3d2a1ab

Please sign in to comment.