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

Remove XHR support from Flight #26827

Merged
merged 1 commit into from
Jun 3, 2023
Merged

Conversation

sebmarkbage
Copy link
Collaborator

We currently support passing an XHR request to Flight for broader compat and possibly better perf than fetch(). However, it's a little tricky because ideally the RSC protocol is really meant to support binary data too. XHR does support binary but it doesn't support it while also streaming.

We could maybe support this only when you know it's going to be only text streams but it has some limitations in how we can encode separators if we can't use binary.

Nobody is really asking for this so we might as well delete it.

@facebook-github-bot facebook-github-bot added CLA Signed React Core Team Opened by a member of the React Core Team labels May 17, 2023
Copy link
Member

@kassens kassens left a comment

Choose a reason for hiding this comment

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

Makes sense.

In Relay we used an Observable interface that one could implement for any kind of transport. Iff we need something like this back in the future, maybe that'd be an option too. (a built-in standards might be enough though)

@sebmarkbage
Copy link
Collaborator Author

You still can build a custom one for whatever network transport from the "Config" just like you can build custom renderers. It's just static dependency injection at the build level instead of runtime for best perf.

@react-sizebot
Copy link

Comparing: d7a98a5...c7b6605

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.23 kB 164.23 kB = 51.77 kB 51.77 kB
oss-experimental/react-dom/cjs/react-dom.production.min.js = 171.59 kB 171.59 kB = 53.98 kB 53.98 kB
facebook-www/ReactDOM-prod.classic.js = 570.55 kB 570.55 kB = 100.66 kB 100.66 kB
facebook-www/ReactDOM-prod.modern.js = 554.29 kB 554.29 kB = 97.84 kB 97.84 kB
oss-experimental/react-server-dom-webpack/umd/react-server-dom-webpack-client.browser.development.js = 54.34 kB 53.14 kB = 12.88 kB 12.67 kB
oss-stable-semver/react-server-dom-webpack/umd/react-server-dom-webpack-client.browser.development.js = 54.34 kB 53.14 kB = 12.88 kB 12.67 kB
oss-stable/react-server-dom-webpack/umd/react-server-dom-webpack-client.browser.development.js = 54.34 kB 53.14 kB = 12.88 kB 12.67 kB
oss-experimental/react-server-dom-webpack/cjs/react-server-dom-webpack-client.browser.development.js = 51.13 kB 49.99 kB = 12.67 kB 12.46 kB
oss-stable-semver/react-server-dom-webpack/cjs/react-server-dom-webpack-client.browser.development.js = 51.13 kB 49.99 kB = 12.67 kB 12.46 kB
oss-stable/react-server-dom-webpack/cjs/react-server-dom-webpack-client.browser.development.js = 51.13 kB 49.99 kB = 12.67 kB 12.46 kB
oss-experimental/react-server-dom-webpack/umd/react-server-dom-webpack-client.browser.production.min.js = 10.87 kB 10.42 kB = 4.26 kB 4.10 kB
oss-stable-semver/react-server-dom-webpack/umd/react-server-dom-webpack-client.browser.production.min.js = 10.87 kB 10.42 kB = 4.26 kB 4.10 kB
oss-stable/react-server-dom-webpack/umd/react-server-dom-webpack-client.browser.production.min.js = 10.87 kB 10.42 kB = 4.26 kB 4.10 kB
oss-experimental/react-server-dom-webpack/cjs/react-server-dom-webpack-client.browser.production.min.js = 10.61 kB 10.15 kB = 4.15 kB 4.01 kB
oss-stable-semver/react-server-dom-webpack/cjs/react-server-dom-webpack-client.browser.production.min.js = 10.61 kB 10.15 kB = 4.15 kB 4.01 kB
oss-stable/react-server-dom-webpack/cjs/react-server-dom-webpack-client.browser.production.min.js = 10.61 kB 10.15 kB = 4.15 kB 4.01 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-server-dom-webpack/umd/react-server-dom-webpack-client.browser.development.js = 54.34 kB 53.14 kB = 12.88 kB 12.67 kB
oss-stable-semver/react-server-dom-webpack/umd/react-server-dom-webpack-client.browser.development.js = 54.34 kB 53.14 kB = 12.88 kB 12.67 kB
oss-stable/react-server-dom-webpack/umd/react-server-dom-webpack-client.browser.development.js = 54.34 kB 53.14 kB = 12.88 kB 12.67 kB
oss-experimental/react-server-dom-webpack/cjs/react-server-dom-webpack-client.browser.development.js = 51.13 kB 49.99 kB = 12.67 kB 12.46 kB
oss-stable-semver/react-server-dom-webpack/cjs/react-server-dom-webpack-client.browser.development.js = 51.13 kB 49.99 kB = 12.67 kB 12.46 kB
oss-stable/react-server-dom-webpack/cjs/react-server-dom-webpack-client.browser.development.js = 51.13 kB 49.99 kB = 12.67 kB 12.46 kB
oss-experimental/react-server-dom-webpack/umd/react-server-dom-webpack-client.browser.production.min.js = 10.87 kB 10.42 kB = 4.26 kB 4.10 kB
oss-stable-semver/react-server-dom-webpack/umd/react-server-dom-webpack-client.browser.production.min.js = 10.87 kB 10.42 kB = 4.26 kB 4.10 kB
oss-stable/react-server-dom-webpack/umd/react-server-dom-webpack-client.browser.production.min.js = 10.87 kB 10.42 kB = 4.26 kB 4.10 kB
oss-experimental/react-server-dom-webpack/cjs/react-server-dom-webpack-client.browser.production.min.js = 10.61 kB 10.15 kB = 4.15 kB 4.01 kB
oss-stable-semver/react-server-dom-webpack/cjs/react-server-dom-webpack-client.browser.production.min.js = 10.61 kB 10.15 kB = 4.15 kB 4.01 kB
oss-stable/react-server-dom-webpack/cjs/react-server-dom-webpack-client.browser.production.min.js = 10.61 kB 10.15 kB = 4.15 kB 4.01 kB

Generated by 🚫 dangerJS against c7b6605

Copy link

@rj1 rj1 left a comment

Choose a reason for hiding this comment

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

good idea

@sebmarkbage sebmarkbage merged commit e6fae30 into facebook:main Jun 3, 2023
sebmarkbage added a commit that referenced this pull request Jun 10, 2023
Follow up to #26827.

These can't include binary data and we don't really have any use cases
that really require these to already be strings.

When the stream is encoded inside another protocol - such as HTML we
need a different format that encode binary offsets and binary data.
EdisonVan pushed a commit to EdisonVan/react that referenced this pull request Apr 15, 2024
We currently support passing an XHR request to Flight for broader compat
and possibly better perf than `fetch()`. However, it's a little tricky
because ideally the RSC protocol is really meant to support binary data
too. XHR does support binary but it doesn't support it while also
streaming.

We could maybe support this only when you know it's going to be only
text streams but it has some limitations in how we can encode separators
if we can't use binary.

Nobody is really asking for this so we might as well delete it.
EdisonVan pushed a commit to EdisonVan/react that referenced this pull request Apr 15, 2024
Follow up to facebook#26827.

These can't include binary data and we don't really have any use cases
that really require these to already be strings.

When the stream is encoded inside another protocol - such as HTML we
need a different format that encode binary offsets and binary data.
bigfootjon pushed a commit that referenced this pull request Apr 18, 2024
We currently support passing an XHR request to Flight for broader compat
and possibly better perf than `fetch()`. However, it's a little tricky
because ideally the RSC protocol is really meant to support binary data
too. XHR does support binary but it doesn't support it while also
streaming.

We could maybe support this only when you know it's going to be only
text streams but it has some limitations in how we can encode separators
if we can't use binary.

Nobody is really asking for this so we might as well delete it.

DiffTrain build for commit e6fae30.
bigfootjon pushed a commit that referenced this pull request Apr 18, 2024
Follow up to #26827.

These can't include binary data and we don't really have any use cases
that really require these to already be strings.

When the stream is encoded inside another protocol - such as HTML we
need a different format that encode binary offsets and binary data.

DiffTrain build for commit ce6842d.
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.

6 participants