Skip to content

Test case for stack overflow in ReactFizzServer #25971

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.

Sign up for GitHub

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

Merged
tyao1 merged 2 commits into from
Jan 11, 2023
Merged

Test case for stack overflow in ReactFizzServer #25971

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9e4bf84
Regression test for stack overflow in Fizz
tyao1 Jan 7, 2023
4029c74
Avoid creating new component and fix test in build
tyao1 Jan 10, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 27 additions & 0 deletions packages/react-dom/src/__tests__/ReactDOMFizzShellHydration-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -283,4 +283,31 @@ describe('ReactDOMFizzShellHydration', () => {
]);
expect(container.textContent).toBe('New screen');
});

test('TODO: A large component stack causes SSR to stack overflow', async () => {
spyOnDevAndProd(console, 'error');

function createNestedComponent(depth: number) {
if (depth <= 0) {
return function Leaf() {
return <AsyncText text="Shell" />;
};
}
const NextComponent = createNestedComponent(depth - 1);
function Component() {
return <NextComponent />;
}
return Component;
}
const NestedComponent = createNestedComponent(1500);
Copy link
Copy Markdown
Member

@kassens kassens Jan 10, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you can just make this a single component. Your current implementation returns a different instance on every re-render even for the same depth which is maybe not ideal.

Suggested change
function createNestedComponent(depth: number) {
if (depth <= 0) {
return function Leaf() {
return <AsyncText text="Shell" />;
};
}
const NextComponent = createNestedComponent(depth - 1);
function Component() {
return <NextComponent />;
}
return Component;
}
const NestedComponent = createNestedComponent(1500);
function NestedComponent({depth}: {depth: number}) {
if (depth <= 0) {
return null;
}
return <NestedComponent depth={depth - 1} />;
}

Copy link
Copy Markdown
Contributor Author

@tyao1 tyao1 Jan 10, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great catch!


// Server render
await serverAct(async () => {
ReactDOMFizzServer.renderToPipeableStream(<NestedComponent />);
});
expect(console.error).toHaveBeenCalledTimes(1);
expect(console.error.calls.argsFor(0)[0].toString()).toBe(
'RangeError: Maximum call stack size exceeded',
);
});
});