stash the component stack on the thrown value and reuse #25790

gnoff · 2022-12-02T22:21:21Z

ErrorBoundaries are currently not fully composable. The reason is if you decide your boundary cannot handle a particular error and rethrow it to higher boundary the React runtime does not understand that this throw is a forward and it recreates the component stack from the Boundary position. This loses fidelity and is especially bad if the boundary is limited it what it handles and high up in the component tree.

This implementation uses a WeakMap to store component stacks for values that are objects. If an error is rethrown from an ErrorBoundary the stack will be pulled from the map if it exists. This doesn't work for thrown primitives but this is uncommon and stashing the stack on the primitive also wouldn't work

sizebot · 2022-12-02T22:28:04Z

Comparing: f0534ae...d1d6184

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.min.js	+0.14%	154.40 kB	154.61 kB	+0.11%	49.00 kB	49.05 kB
oss-experimental/react-dom/cjs/react-dom.production.min.js	+0.13%	156.32 kB	156.53 kB	+0.13%	49.65 kB	49.71 kB
facebook-www/ReactDOM-prod.classic.js	+0.07%	533.40 kB	533.79 kB	+0.13%	94.99 kB	95.12 kB
facebook-www/ReactDOM-prod.modern.js	+0.07%	518.50 kB	518.89 kB	+0.15%	92.80 kB	92.94 kB
facebook-www/ReactDOMForked-prod.classic.js	+0.07%	533.40 kB	533.79 kB	+0.13%	94.99 kB	95.12 kB

Significant size changes

Includes any change greater than 0.2%:

Expand to show

Name	+/-	Base	Current	+/- gzip	Base gzip	Current gzip
oss-stable-semver/react-test-renderer/umd/react-test-renderer.production.min.js	+0.23%	100.44 kB	100.67 kB	+0.31%	31.16 kB	31.26 kB
oss-stable/react-test-renderer/umd/react-test-renderer.production.min.js	+0.23%	100.46 kB	100.69 kB	+0.30%	31.16 kB	31.26 kB
oss-experimental/react-test-renderer/umd/react-test-renderer.production.min.js	+0.23%	100.52 kB	100.75 kB	+0.30%	31.19 kB	31.28 kB
oss-stable-semver/react-test-renderer/cjs/react-test-renderer.production.min.js	+0.23%	100.20 kB	100.43 kB	+0.22%	30.74 kB	30.81 kB
oss-stable/react-test-renderer/cjs/react-test-renderer.production.min.js	+0.23%	100.22 kB	100.45 kB	+0.21%	30.74 kB	30.81 kB
oss-experimental/react-test-renderer/cjs/react-test-renderer.production.min.js	+0.23%	100.28 kB	100.51 kB	+0.21%	30.78 kB	30.84 kB
oss-stable-semver/react-art/cjs/react-art.production.min.js	+0.23%	92.33 kB	92.54 kB	+0.19%	28.43 kB	28.49 kB
oss-stable/react-art/cjs/react-art.production.min.js	+0.23%	92.35 kB	92.56 kB	+0.19%	28.43 kB	28.49 kB
oss-experimental/react-art/cjs/react-art.production.min.js	+0.22%	93.77 kB	93.98 kB	+0.19%	28.87 kB	28.92 kB

Generated by 🚫 dangerJS against d1d6184

sebmarkbage · 2024-02-16T01:06:25Z

packages/react-reconciler/src/ReactCapturedValue.js

@@ -24,10 +25,24 @@ export function createCapturedValueAtFiber<T>(
 ): CapturedValue<T> {
  // If the value is an error, call this function immediately after it is thrown
  // so the stack is accurate.
+  let stack;
+  if (value != null && hasOwnProperty.call(value, '_componentStack')) {


We probably should use a WeakMap instead to avoid any conflicts on the object.

sebmarkbage

WeakMap and then good to land.

react-sizebot · 2024-02-16T04:28:53Z

Comparing: fea900e...3408207

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.min.js	+0.10%	176.83 kB	177.01 kB	+0.11%	55.11 kB	55.17 kB
oss-experimental/react-dom/cjs/react-dom.production.min.js	+0.10%	178.83 kB	179.00 kB	+0.09%	55.69 kB	55.74 kB
facebook-www/ReactDOM-prod.classic.js	+0.07%	592.18 kB	592.58 kB	+0.11%	104.43 kB	104.54 kB
facebook-www/ReactDOM-prod.modern.js	+0.07%	575.46 kB	575.87 kB	+0.12%	101.43 kB	101.55 kB
test_utils/ReactAllWarnings.js	Deleted	66.25 kB	0.00 kB	Deleted	16.31 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	66.25 kB	0.00 kB	Deleted	16.31 kB	0.00 kB

Generated by 🚫 dangerJS against 3408207

…ed a component stack

gnoff · 2024-02-16T04:43:44Z

packages/react-reconciler/src/ReactCapturedValue.js

-export function createCapturedValue<T>(
-  value: T,
+export function createCapturedValueFromError(
+  value: Error,


this was already being used on to construct add hoc captured values for the recoverable error pathway. It doesn't need value generality and this helps us avoid type checking the value before deciding if the stack needs to go in the map

gnoff · 2024-02-16T04:44:28Z

packages/react-reconciler/src/ReactCapturedValue.js

-): CapturedValue<T> {
+): CapturedValue<Error> {
+  if (typeof stack === 'string') {
+    CapturedStacks.set(value, stack);


The way this is used today we don't really need to capture the stack in the map because these errors aren't bubbling up during a render. However it could be used in other ways in the future so to keep it correct I'm stashing the stack here if it is provided.

ErrorBoundaries are currently not fully composable. The reason is if you decide your boundary cannot handle a particular error and rethrow it to higher boundary the React runtime does not understand that this throw is a forward and it recreates the component stack from the Boundary position. This loses fidelity and is especially bad if the boundary is limited it what it handles and high up in the component tree. This implementation uses a WeakMap to store component stacks for values that are objects. If an error is rethrown from an ErrorBoundary the stack will be pulled from the map if it exists. This doesn't work for thrown primitives but this is uncommon and stashing the stack on the primitive also wouldn't work DiffTrain build for [a9cc325](a9cc325)

@acdlite

ErrorBoundaries are currently not fully composable. The reason is if you decide your boundary cannot handle a particular error and rethrow it to higher boundary the React runtime does not understand that this throw is a forward and it recreates the component stack from the Boundary position. This loses fidelity and is especially bad if the boundary is limited it what it handles and high up in the component tree. This implementation uses a WeakMap to store component stacks for values that are objects. If an error is rethrown from an ErrorBoundary the stack will be pulled from the map if it exists. This doesn't work for thrown primitives but this is uncommon and stashing the stack on the primitive also wouldn't work

ErrorBoundaries are currently not fully composable. The reason is if you decide your boundary cannot handle a particular error and rethrow it to higher boundary the React runtime does not understand that this throw is a forward and it recreates the component stack from the Boundary position. This loses fidelity and is especially bad if the boundary is limited it what it handles and high up in the component tree. This implementation uses a WeakMap to store component stacks for values that are objects. If an error is rethrown from an ErrorBoundary the stack will be pulled from the map if it exists. This doesn't work for thrown primitives but this is uncommon and stashing the stack on the primitive also wouldn't work DiffTrain build for commit a9cc325.

facebook-github-bot added the CLA Signed label Dec 2, 2022

gnoff requested a review from sebmarkbage December 2, 2022 22:24

poteto added the React Core Team Opened by a member of the React Core Team label Dec 7, 2022

sebmarkbage reviewed Feb 16, 2024

View reviewed changes

sebmarkbage approved these changes Feb 16, 2024

View reviewed changes

gnoff force-pushed the retain-error-stacks branch from d1d6184 to 40a2cb9 Compare February 16, 2024 04:25

Retain component stacks when rethrowing an error that already generat…

3408207

…ed a component stack

gnoff force-pushed the retain-error-stacks branch from 40a2cb9 to 3408207 Compare February 16, 2024 04:31

gnoff commented Feb 16, 2024

View reviewed changes

gnoff merged commit a9cc325 into facebook:main Feb 16, 2024
36 checks passed

gnoff deleted the retain-error-stacks branch February 16, 2024 04:44

This was referenced Feb 20, 2024

Bump vendored react versions vercel/next.js#62302

Closed

Upgrade vendored react vercel/next.js#62326

Merged

ztanner mentioned this pull request Mar 19, 2024

(backport) Update React from 60a927d04 to 14898b6a9 vercel/next.js#63442

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

stash the component stack on the thrown value and reuse #25790

stash the component stack on the thrown value and reuse #25790

gnoff commented Dec 2, 2022 •

edited

Loading

sizebot commented Dec 2, 2022 •

edited

Loading

sebmarkbage Feb 16, 2024

sebmarkbage left a comment

react-sizebot commented Feb 16, 2024 •

edited

Loading

gnoff Feb 16, 2024

gnoff Feb 16, 2024

stash the component stack on the thrown value and reuse #25790

stash the component stack on the thrown value and reuse #25790

Conversation

gnoff commented Dec 2, 2022 • edited Loading

sizebot commented Dec 2, 2022 • edited Loading

Critical size changes

Significant size changes

sebmarkbage Feb 16, 2024

Choose a reason for hiding this comment

sebmarkbage left a comment

Choose a reason for hiding this comment

react-sizebot commented Feb 16, 2024 • edited Loading

Critical size changes

Significant size changes

gnoff Feb 16, 2024

Choose a reason for hiding this comment

gnoff Feb 16, 2024

Choose a reason for hiding this comment

gnoff commented Dec 2, 2022 •

edited

Loading

sizebot commented Dec 2, 2022 •

edited

Loading

react-sizebot commented Feb 16, 2024 •

edited

Loading