Skip to content
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.

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

Test case for stack overflow in ReactFizzServer #25971

Merged
merged 2 commits into from
Jan 11, 2023
Merged

Test case for stack overflow in ReactFizzServer #25971

merged 2 commits into from
Jan 11, 2023

Conversation

tyao1
Copy link
Contributor

@tyao1 tyao1 commented Jan 7, 2023

No description provided.

@tyao1 tyao1 requested a review from mofeiZ January 7, 2023 03:19
@facebook-github-bot facebook-github-bot added CLA Signed React Core Team Opened by a member of the React Core Team labels Jan 7, 2023
Comment on lines 290 to 302
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
Member

@kassens kassens Jan 10, 2023

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
Contributor Author

Choose a reason for hiding this comment

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

Great catch!

@tyao1 tyao1 merged commit fb324fa into main Jan 11, 2023
@kassens kassens deleted the ty-stack-overflow branch January 11, 2023 00:54
github-actions bot pushed a commit that referenced this pull request Jan 11, 2023
SSR currently stack overflows when the component tree is extremely large

DiffTrain build for [fb324fa](fb324fa)
[View git log for this commit](https://github.com/facebook/react/commits/fb324faf8a3da08212bcc5ea9e3a084dbfa80dad)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed React Core Team Opened by a member of the React Core Team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants