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

Move Flight Fixture to use Middleware instead of WebDevServer #26246

Merged
merged 1 commit into from
Feb 25, 2023

Conversation

sebmarkbage
Copy link
Collaborator

@sebmarkbage sebmarkbage commented Feb 25, 2023

This lets us put it in the same server that would be serving this content in a more real world scenario.

I also de-CRA:ified this a bit by simplifying pieces we don't need.

I have more refactors coming for the SSR pieces but since many are eyeing these fixtures right now I figured I'd push earlier.

The design here is that there are two servers:

  • Global - representing a "CDN" delivering static content to the user, which will also include the SSR server in this set up.
  • Regional - representing something close to the data with low waterfall costs which include the RSC server.

This is just an example.

These are using the "unbundled" strategy for the RSC server just to show a simple case, but an implementation can use a bundled SSR server.

A smart SSR bundler could also put RSC and SSR in the same server and even the same JS environment. It just need to ensure that the module graphs are kept separately - so that the react-server condition is respected. This include react itself. React will start breaking if this isn't respected because the runtime will get the wrong copy of react. Technically, you don't need the entire module graph to be separated. It just needs to be any part of the graph that depends on a fork. Like if "Client A" -> "foo" and "Server B" -> "foo", then it's ok for the module "foo" to be shared. However if "foo" -> "bar", and "bar" is forked by the "react-server" condition, then "foo" also needs to be duplicated in the module graph so that it can get two copies of "bar".

This lets us put it in the same server that would be serving this content
in a more real world scenario.

I also de-CRA:ified this a bit by simplifying pieces we don't need.
@facebook-github-bot facebook-github-bot added CLA Signed React Core Team Opened by a member of the React Core Team labels Feb 25, 2023
@react-sizebot
Copy link

Comparing: 212b89f...a05633d

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 = 154.55 kB 154.55 kB = 48.79 kB 48.79 kB
oss-experimental/react-dom/cjs/react-dom.production.min.js = 156.54 kB 156.54 kB = 49.45 kB 49.45 kB
facebook-www/ReactDOM-prod.classic.js = 531.11 kB 531.11 kB = 94.64 kB 94.64 kB
facebook-www/ReactDOM-prod.modern.js = 515.07 kB 515.07 kB = 92.17 kB 92.17 kB

Significant size changes

Includes any change greater than 0.2%:

(No significant changes)

Generated by 🚫 dangerJS against a05633d

@sophiebits
Copy link
Collaborator

Like if "Client A" -> "foo" and "Server B" -> "foo", then it's ok for the module "foo" to be shared.

No worries about "foo" being stateful?

@sebmarkbage sebmarkbage merged commit e7d7d4c into facebook:main Feb 25, 2023
github-actions bot pushed a commit that referenced this pull request Feb 25, 2023
This lets us put it in the same server that would be serving this
content in a more real world scenario.

I also de-CRA:ified this a bit by simplifying pieces we don't need.

I have more refactors coming for the SSR pieces but since many are
eyeing these fixtures right now I figured I'd push earlier.

The design here is that there are two servers:

- Global - representing a "CDN" which will also include the SSR server.
- Regional - representing something close to the data with low waterfall
costs which include the RSC server.

This is just an example.

These are using the "unbundled" strategy for the RSC server just to show
a simple case, but an implementation can use a bundled SSR server.

A smart SSR bundler could also put RSC and SSR in the same server and
even the same JS environment. It just need to ensure that the module
graphs are kept separately - so that the `react-server` condition is
respected. This include `react` itself. React will start breaking if
this isn't respected because the runtime will get the wrong copy of
`react`. Technically, you don't need the *entire* module graph to be
separated. It just needs to be any part of the graph that depends on a
fork. Like if "Client A" -> "foo" and "Server B" -> "foo", then it's ok
for the module "foo" to be shared. However if "foo" -> "bar", and "bar"
is forked by the "react-server" condition, then "foo" also needs to be
duplicated in the module graph so that it can get two copies of "bar".

DiffTrain build for [e7d7d4c](e7d7d4c)
@sebmarkbage
Copy link
Collaborator Author

No worries about "foo" being stateful?

You shouldn't really have a shared stateful dependency between Client and Server in this model. There's no "peerDependency" in this model. Outside the global coordinator that has to transfer the Server stream to the Client SSR.

In this regard it's not really different from two node_modules being deduped or not deduped depending on versions of those dependencies.

@unstubbable
Copy link
Contributor

Thanks, @sebmarkbage. I'm one those who's eyeing these fixtures. 😊 Would love to see a version of the regional server that's bundled (for edge) instead of using the (experimental) node loader, maybe as a future iteration.

@sebmarkbage
Copy link
Collaborator Author

@unstubbable You can take a look at Next.js for now. It's way more complicated, but some of that is inherent. There's a lot of complexity that goes into making that ecosystem compatible. For Node.js it's even more complicated because there's both bundled parts and unbundled parts of the graph with an ESM->CJS interop layer. Some of that is really out of the scope of React.

There's also CSS which is another story.

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

6 participants