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

Implement onError signature for renderToMarkup #30170

Merged
merged 3 commits into from
Jul 2, 2024

Conversation

sebmarkbage
Copy link
Collaborator

@sebmarkbage sebmarkbage commented Jul 1, 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.

@sebmarkbage sebmarkbage requested a review from eps1lon July 1, 2024 16:03
Copy link

vercel bot commented Jul 1, 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 7:17pm

@react-sizebot
Copy link

react-sizebot commented Jul 1, 2024

Comparing: 100dfd7...0db9d71

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.05% 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.05% 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
oss-experimental/react-html/cjs/react-html.react-server.development.js +0.22% 484.10 kB 485.16 kB +0.26% 87.66 kB 87.88 kB
test_utils/ReactAllWarnings.js Deleted 62.88 kB 0.00 kB Deleted 15.69 kB 0.00 kB

Generated by 🚫 dangerJS against 0db9d71

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.
Inside the onError event, the current owner stack is active but it's active
to the client runtime but we can access it from server runtime which is
what React.captureOwnerStack() points to.

So we need to forward this.
@eps1lon
Copy link
Collaborator

eps1lon commented Jul 2, 2024

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.

What's an example of a Fight error that's not observable?

@sebmarkbage
Copy link
Collaborator Author

What's an example of a Fight error that's not observable?

Passing a Promise that rejects into a property that ends up not used like an unused Suspense prop. Another case might be erroring side a fallback of a Suspense boundary if it ends up never getting attempted.

@sebmarkbage sebmarkbage merged commit 309e146 into facebook:main Jul 2, 2024
139 checks passed
sebmarkbage added a commit that referenced this pull request Jul 2, 2024
Stacked on #30170.

This lets us track Server Component parent stacks in Fizz which also
lets us track the correct owner stack for lazy.

In Fiber we're careful not to make any DEV only fibers but since the
ReactFizzComponentStack data structures just exist for debug meta data
anyway we can just expand on that.
github-actions bot pushed a commit that referenced this pull request Jul 2, 2024
Stacked on #30170.

This lets us track Server Component parent stacks in Fizz which also
lets us track the correct owner stack for lazy.

In Fiber we're careful not to make any DEV only fibers but since the
ReactFizzComponentStack data structures just exist for debug meta data
anyway we can just expand on that.

DiffTrain build for [cfb8945](cfb8945)
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