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] Allow passing a reason to abortStream #26992

Merged
merged 2 commits into from
Sep 20, 2023

Conversation

KarimP
Copy link
Contributor

@KarimP KarimP commented Jun 22, 2023

Summary

Currently ReactFizzServer.abort allows you to pass in the a reason error, which then gets passed to the onError handler for each task that ends up getting aborted. This adds in the ability to pass down that same reason error to ReactDOMServerFB.abortStream as well.

How did you test this change?

Added a test case to ReactDOMServerFB-test.internal.js

@react-sizebot
Copy link

react-sizebot commented Jun 22, 2023

Comparing: c8deb5d...0ab4828

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 = 164.56 kB 164.56 kB = 51.83 kB 51.83 kB
oss-experimental/react-dom/cjs/react-dom.production.min.js = 171.97 kB 171.97 kB = 54.06 kB 54.06 kB
facebook-www/ReactDOM-prod.classic.js = 571.74 kB 571.74 kB = 100.83 kB 100.83 kB
facebook-www/ReactDOM-prod.modern.js = 555.52 kB 555.52 kB = 98.02 kB 98.02 kB

Significant size changes

Includes any change greater than 0.2%:

(No significant changes)

Generated by 🚫 dangerJS against 0ab4828

@KarimP KarimP marked this pull request as ready for review June 22, 2023 17:24
Copy link
Contributor

@mofeiZ mofeiZ left a comment

Choose a reason for hiding this comment

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

Changes make sense to me! It looks like all other DOM server apis allow passing in a reason (e.g. ReactDOMFizzServerNode).

Bundle size changes might be too small to show up from DangerJS. ReactDOMServerStreaming should contain this change though.

@mofeiZ mofeiZ merged commit 1f49366 into facebook:main Sep 20, 2023
2 checks passed
github-actions bot pushed a commit that referenced this pull request Sep 20, 2023
## Summary

Currently `ReactFizzServer.abort` allows you to pass in the a `reason`
error, which then gets passed to the `onError` handler for each task
that ends up getting aborted. This adds in the ability to pass down that
same `reason` error to `ReactDOMServerFB.abortStream` as well.

## How did you test this change?

Added a test case to ReactDOMServerFB-test.internal.js

DiffTrain build for [1f49366](1f49366)
EdisonVan pushed a commit to EdisonVan/react that referenced this pull request Apr 15, 2024
## Summary

Currently `ReactFizzServer.abort` allows you to pass in the a `reason`
error, which then gets passed to the `onError` handler for each task
that ends up getting aborted. This adds in the ability to pass down that
same `reason` error to `ReactDOMServerFB.abortStream` as well.

## How did you test this change?

Added a test case to ReactDOMServerFB-test.internal.js
bigfootjon pushed a commit that referenced this pull request Apr 18, 2024
## Summary

Currently `ReactFizzServer.abort` allows you to pass in the a `reason`
error, which then gets passed to the `onError` handler for each task
that ends up getting aborted. This adds in the ability to pass down that
same `reason` error to `ReactDOMServerFB.abortStream` as well.

## How did you test this change?

Added a test case to ReactDOMServerFB-test.internal.js

DiffTrain build for commit 1f49366.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants