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] Add FB specific streaming API and build #21337

Merged
merged 2 commits into from Apr 22, 2021

Conversation

sebmarkbage
Copy link
Collaborator

@sebmarkbage sebmarkbage commented Apr 22, 2021

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.

@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
Copy link

sizebot commented Apr 22, 2021

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

bvaughn commented Sep 14, 2021

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:

let files = packageJSON.files;
if (!Array.isArray(files)) {
throw new Error('expected all package.json files to contain a files field');
}

The build script also throws if there's no README file, since it tries to copy unconditionally:

asyncCopyTo(
`packages/${name}/README.md`,
`build/node_modules/${name}/README.md`
),

Are we filtering out private packages somewhere?

@sebmarkbage
Copy link
Collaborator Author

sebmarkbage commented Sep 14, 2021

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.

const builtPackageFolders = readdirSync('build/node_modules').filter(

@bvaughn
Copy link
Contributor

bvaughn commented Sep 15, 2021

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.

None yet

5 participants