Skip to content

[Fizz] Add FB specific streaming API and build#21337

Merged
sebmarkbage merged 2 commits into
react:masterfrom
sebmarkbage:fizzfb
Apr 22, 2021
Merged

[Fizz] Add FB specific streaming API and build#21337
sebmarkbage merged 2 commits into
react:masterfrom
sebmarkbage:fizzfb

Conversation

@sebmarkbage

@sebmarkbage sebmarkbage commented Apr 22, 2021

Copy link
Copy Markdown
Contributor

We don't have either Browser nor Node streams at FB WWW. This adds a specific ServerStreamConfig and exported API for this purpose. It's not necessarily optimal. E.g. this doesn't take advantage of back pressure, sharing buffers with the underlying sink and doesn't support TypedArrays. It's just closest to what we already have but we can iterate on this independently.

We also happen to import React with custom builds instead of npm. So I build it separately without an entry point.

It's kinds of similar to what we do with Flight for Relay so I just added it under that folder and it's flow typed under the "dom-relay" flag to avoid having too many Flow passes.

I made a new build that builds to ReactDOMServer.js but only in EXPERIMENTAL which is actually "modern" builds. So the net effect will be that ReactDOMServer.modern.js is Fizz.

I plan on moving Fizz onto the main react-dom/server export but it's difficult because we need it to export multiple builds which goes against the grain of our system atm. But the idea is to export both and then remove legacy. So this is just jumping ahead to the last step for FB.

This structure probably doesn't make much sense overall but it is what it is.

@sebmarkbage sebmarkbage requested a review from gaearon April 22, 2021 22:35
@facebook-github-bot facebook-github-bot added React Core Team Opened by a member of the React Core Team CLA Signed labels Apr 22, 2021
@sizebot

sizebot commented Apr 22, 2021

Copy link
Copy Markdown

Comparing: c3cb2c2...05075f7

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 = 122.72 kB 122.72 kB = 39.39 kB 39.39 kB
oss-experimental/react-dom/cjs/react-dom.production.min.js = 129.30 kB 129.30 kB = 41.47 kB 41.48 kB
facebook-www/ReactDOM-prod.classic.js = 412.22 kB 412.22 kB = 76.23 kB 76.24 kB
facebook-www/ReactDOM-prod.modern.js = 400.29 kB 400.29 kB = 74.32 kB 74.32 kB
facebook-www/ReactDOMForked-prod.classic.js = 412.22 kB 412.22 kB = 76.24 kB 76.24 kB
facebook-www/ReactDOMServer-prod.modern.js +50.86% 47.54 kB 71.72 kB +33.92% 11.06 kB 14.81 kB
facebook-www/ReactDOMServer-dev.modern.js +38.75% 145.85 kB 202.36 kB +27.27% 37.44 kB 47.65 kB

Significant size changes

Includes any change greater than 0.2%:

Expand to show
Name +/- Base Current +/- gzip Base gzip Current gzip
facebook-www/ReactDOMServer-prod.modern.js +50.86% 47.54 kB 71.72 kB +33.92% 11.06 kB 14.81 kB
facebook-www/ReactDOMServer-dev.modern.js +38.75% 145.85 kB 202.36 kB +27.27% 37.44 kB 47.65 kB

Generated by 🚫 dangerJS against 05075f7

@sebmarkbage sebmarkbage merged commit 709f948 into react:master Apr 22, 2021
facebook-github-bot pushed a commit to react/react-native that referenced this pull request Apr 28, 2021
Summary:
This sync includes the following changes:
- **[9a25916](react/react@9a2591681 )**: Fix export //<Sebastian Markbage>//
- **[4a8deb0](react/react@4a8deb083 )**: Switch the isPrimaryRender flag based on the stream config ([#21357](react/react#21357)) //<Sebastian Markbåge>//
- **[bd4f056](react/react@bd4f056a3 )**: [Fizz] Implement lazy components and nodes ([#21355](react/react#21355)) //<Sebastian Markbåge>//
- **[fc33f12](react/react@fc33f12bd )**: Remove unstable scheduler/tracing API ([#20037](react/react#20037)) //<Brian Vaughn>//
- **[7212383](react/react@721238394 )**: Enable strict effects mode for React Native Facebook builds ([#21354](react/react#21354)) //<Brian Vaughn>//
- **[4874042](react/react@48740429b )**: Expiration: Do nothing except disable time slicing ([#21345](react/react#21345)) //<Andrew Clark>//
- **[0f5ebf3](react/react@0f5ebf366 )**: Delete unreferenced type ([#21343](react/react#21343)) //<Andrew Clark>//
- **[9cd52b2](react/react@9cd52b27f )**: Restore context after an error happens ([#21341](react/react#21341)) //<Sebastian Markbåge>//
- **[ad09175](react/react@ad091759a )**: Revert "Emit reactroot attribute on the first element we discover ([#21154](react/react#21154))" ([#21340](react/react#21340)) //<Sebastian Markbåge>//
- **[709f948](react/react@709f94841 )**: [Fizz] Add FB specific streaming API and build ([#21337](react/react#21337)) //<Sebastian Markbåge>//
- **[e8cdce4](react/react@e8cdce40d )**: Don't flush sync at end of discreteUpdates ([#21327](react/react#21327)) //<Andrew Clark>//
- **[a155860](react/react@a15586001 )**: Fix: Don't flush discrete at end of batchedUpdates ([#21229](react/react#21229)) //<Andrew Clark>//
- **[89847bf](react/react@89847bf6e )**: Continuous updates should interrupt transitions ([#21323](react/react#21323)) //<Andrew Clark>//
- **[ef37d55](react/react@ef37d55b6 )**: Use performConcurrentWorkOnRoot for "sync default" ([#21322](react/react#21322)) //<Andrew Clark>//

Changelog:
[General][Changed] - React Native sync for revisions a632f7d...2a7bb41

jest_e2e[run_all_tests]

Reviewed By: JoshuaGross

Differential Revision: D28063006

fbshipit-source-id: 7e3535f80961706863b6c2188ee44b5796b2f000
koto pushed a commit to koto/react that referenced this pull request Jun 15, 2021
@bvaughn

bvaughn commented Sep 14, 2021

Copy link
Copy Markdown
Contributor

I wonder how this PR was able to land given that packages/react-server-dom-relay/package.json doesn't define a files field and the build scripts seem to enforce this:
https://github.com/facebook/react/blob/50263d3273b6fc983acc5b0fd52e670399b248b1/scripts/rollup/packaging.js#L149-L152

The build script also throws if there's no README file, since it tries to copy unconditionally:
https://github.com/facebook/react/blob/50263d3273b6fc983acc5b0fd52e670399b248b1/scripts/rollup/packaging.js#L193-L196

Are we filtering out private packages somewhere?

@sebmarkbage

sebmarkbage commented Sep 14, 2021

Copy link
Copy Markdown
Contributor Author

Seems we don’t get there if there wasn’t a node_modules built which there wouldn’t be for FB-only builds. Like if there’s no NODE_PROD.

https://github.com/facebook/react/blob/cae635054e17a6f107a39d328649137b83f25972/scripts/rollup/packaging.js#L213

@bvaughn

bvaughn commented Sep 15, 2021

Copy link
Copy Markdown
Contributor

Hm. When I build all packages locally, build/node_module includes 'react-server-dom-relay' (which is why I noticed this in the first place).

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.

5 participants