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

[Fizz] Include a component stack in prod but only lazily generate it #30132

Merged
merged 1 commit into from
Jul 2, 2024

Conversation

sebmarkbage
Copy link
Collaborator

@sebmarkbage sebmarkbage commented Jun 29, 2024

When we added component stacks to Fizz in prod it severely slowed down common cases where you intentionally are throwing error for purposes of client rendering. Our parent component stack generation is very slow since call components with fake errors to generate them.

Therefore we disabled them in prod but included them in prerenders. #27850

However, we still kept generating data structures for them and the code still exists there for the prerenders. We could stop generating the data structures which are not completely free but also not crazy bad.

What we can do instead is just lazily generate the component stacks. This is in fact what plain stacks do anyway. This doesn't work as well in Fiber because the data structures are live but on the server they're immutable so it's fine to do it later as well.

That way you can choose to not read this getter for intentionally thrown errors - after inspecting the Error object - yet still get component stacks in prod for other errors.

Copy link

vercel bot commented Jun 29, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
react-compiler-playground ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jul 1, 2024 4:29pm

@react-sizebot
Copy link

react-sizebot commented Jun 29, 2024

Comparing: 100dfd7...476f6d0

Critical size changes

Includes critical production bundles, as well as any change greater than 2%:

Name +/- Base Current +/- gzip Base gzip Current gzip
oss-stable/react-dom/cjs/react-dom.production.js = 6.68 kB 6.68 kB +0.11% 1.83 kB 1.83 kB
oss-stable/react-dom/cjs/react-dom-client.production.js = 497.99 kB 497.99 kB = 89.27 kB 89.27 kB
oss-experimental/react-dom/cjs/react-dom.production.js = 6.69 kB 6.69 kB +0.11% 1.83 kB 1.83 kB
oss-experimental/react-dom/cjs/react-dom-client.production.js = 502.81 kB 502.81 kB = 89.97 kB 89.97 kB
facebook-www/ReactDOM-prod.classic.js = 597.08 kB 597.08 kB = 105.33 kB 105.33 kB
facebook-www/ReactDOM-prod.modern.js = 571.42 kB 571.42 kB = 101.27 kB 101.27 kB
test_utils/ReactAllWarnings.js Deleted 62.88 kB 0.00 kB Deleted 15.69 kB 0.00 kB

Significant size changes

Includes any change greater than 0.2%:

Expand to show
Name +/- Base Current +/- gzip Base gzip Current gzip
test_utils/ReactAllWarnings.js Deleted 62.88 kB 0.00 kB Deleted 15.69 kB 0.00 kB

Generated by 🚫 dangerJS against 476f6d0

@sebmarkbage sebmarkbage merged commit e60063d into facebook:main Jul 2, 2024
139 checks passed
@sebmarkbage
Copy link
Collaborator Author

Meta can just make sure not to touch this property if a client render happens which I assume is what is done now since it's not used.

github-actions bot pushed a commit that referenced this pull request Jul 2, 2024
…30132)

When we added component stacks to Fizz in prod it severely slowed down
common cases where you intentionally are throwing error for purposes of
client rendering. Our parent component stack generation is very slow
since call components with fake errors to generate them.

Therefore we disabled them in prod but included them in prerenders.
#27850

However, we still kept generating data structures for them and the code
still exists there for the prerenders. We could stop generating the data
structures which are not completely free but also not crazy bad.

What we can do instead is just lazily generate the component stacks.
This is in fact what plain stacks do anyway. This doesn't work as well
in Fiber because the data structures are live but on the server they're
immutable so it's fine to do it later as well.

That way you can choose to not read this getter for intentionally thrown
errors - after inspecting the Error object - yet still get component
stacks in prod for other errors.

DiffTrain build for [e60063d](e60063d)
sebmarkbage added a commit that referenced this pull request Jul 2, 2024
Stacked on #30132.

This way we can get parent and owner stacks from the error.

This forces us to confront multiple errors and whether or not a Flight
error that ends up being unobservable needs to really reject the render.

This implements stashing of Flight errors with a digest and only errors
if they end up erroring the Fizz render too. At this point they'll have
parent stacks so we can surface those.
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.

4 participants