[Flight][Fizz][Fiber] Refactor configs #26571
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
facebook-github-bot
added
CLA Signed
React Core Team
sebmarkbage
reviewed
Apr 8, 2023
| @@ -0,0 +1,11 @@ | |||
| /** | |||
There was a problem hiding this comment.
What will this file be used for?
The naming convention seems like it should be ReactFlightClientHostConfigDOM if you look at its siblings:
ReactFlightClientHostConfigBrowser (runtime)
ReactFlightClientHostConfigStream (channel)
ReactFlightClientHostConfigDOM (format)
The difference between Browser and DOM here is subtle though.
Should these be split between the Flight client running in Fizz and client-side?
There was a problem hiding this comment.
In my head the *Browser and *Node files were still describing the server runtime and would be appropriate to use for Native and other hosts.
ReactFlight is to ReactFiber
ReactDOMFlight is to ReactDOM in my naming scheme
I could use the naming scheme you suggest but it would still be appropriate to keep the file in react-dom-bindings project. And since it is not a sibling file of the other two I figured it didn't need to follow the same structure
|
We can probably rename FormatConfig too. I wonder if we just drop the "Host" part and just call everything Config. |
|
@sebmarkbage I was considering adding |
gnoff
changed the title
gnoff
force-pushed
the
flightconfig
branch
3 times, most recently
from
9f964a9 to
621590c
Compare
|
@sebmarkbage did a much bigger filename refactor. I'll land as separate commits so git has an easier time tracking filename changes |
Normalize ReactFligthClientConfig and related files
…rom Fizz. Historically the ServerFormatConfigs were supposed to be generic enough to be used by Fizz and Flight. However with the addition of features like Float the configs have evolved to be more specific to the renderer. We may want to get back to a place where there is a pure FormatConfig which can be shared but for now we are embracing the fact that these runtimes need very different things and DCE cannot adequately remove the unused stuff for Fizz when pulling this dep into Flight so we are going to fork the configs and just maintain separate ones. At first the Flight config will be almost empty but once Float support in Flight lands it will have a more complex implementation Additionally this commit normalizes the component files which make up FlightServerConfig and FlightClientConfig. Now each file that participates starts with ReactFlightServerConfig... and ReactFlightClientConfig...
…r Fiber and not Fizz/Fligh. This better conforms to the naming used in Flight and now Fizz of `ReactFlightServerConfig` and `ReactFizzConfig`
…ribe the configs we customize per runtime like FlightClient, FlightServer, Fizz, and Fiber. This commit generalizes $$$hostconfig to $$$config
gnoff
added a commit
that referenced
this pull request
Apr 10, 2023
First part of #26571 merging separately to help with git history with a lot of file renames
gnoff
added a commit
that referenced
this pull request
Apr 10, 2023
Part of #26571 Implements wiring for Flight to have it's own "HostConfig" from Fizz. Historically the ServerFormatConfigs were supposed to be generic enough to be used by Fizz and Flight. However with the addition of features like Float the configs have evolved to be more specific to the renderer. We may want to get back to a place where there is a pure FormatConfig which can be shared but for now we are embracing the fact that these runtimes need very different things and DCE cannot adequately remove the unused stuff for Fizz when pulling this dep into Flight so we are going to fork the configs and just maintain separate ones. At first the Flight config will be almost empty but once Float support in Flight lands it will have a more complex implementation Additionally this commit normalizes the component files which make up FlightServerConfig and FlightClientConfig. Now each file that participates starts with ReactFlightServerConfig... and ReactFlightClientConfig...
gnoff
added a commit
that referenced
this pull request
Apr 10, 2023
part of #26571 merging separately to improve tracking of file renames
github-actions bot
pushed a commit
that referenced
this pull request
Apr 10, 2023
Part of #26571 Implements wiring for Flight to have it's own "HostConfig" from Fizz. Historically the ServerFormatConfigs were supposed to be generic enough to be used by Fizz and Flight. However with the addition of features like Float the configs have evolved to be more specific to the renderer. We may want to get back to a place where there is a pure FormatConfig which can be shared but for now we are embracing the fact that these runtimes need very different things and DCE cannot adequately remove the unused stuff for Fizz when pulling this dep into Flight so we are going to fork the configs and just maintain separate ones. At first the Flight config will be almost empty but once Float support in Flight lands it will have a more complex implementation Additionally this commit normalizes the component files which make up FlightServerConfig and FlightClientConfig. Now each file that participates starts with ReactFlightServerConfig... and ReactFlightClientConfig... DiffTrain build for [f4f873f](f4f873f)
kassens
pushed a commit
to kassens/react
that referenced
this pull request
Apr 17, 2023
First part of facebook#26571 merging separately to help with git history with a lot of file renames
jerrydev0927
added a commit
to jerrydev0927/react
that referenced
this pull request
Jan 5, 2024
Part of facebook/react#26571 Implements wiring for Flight to have it's own "HostConfig" from Fizz. Historically the ServerFormatConfigs were supposed to be generic enough to be used by Fizz and Flight. However with the addition of features like Float the configs have evolved to be more specific to the renderer. We may want to get back to a place where there is a pure FormatConfig which can be shared but for now we are embracing the fact that these runtimes need very different things and DCE cannot adequately remove the unused stuff for Fizz when pulling this dep into Flight so we are going to fork the configs and just maintain separate ones. At first the Flight config will be almost empty but once Float support in Flight lands it will have a more complex implementation Additionally this commit normalizes the component files which make up FlightServerConfig and FlightClientConfig. Now each file that participates starts with ReactFlightServerConfig... and ReactFlightClientConfig... DiffTrain build for [f4f873f6282e6f2e584990c00fb2aae86db85a8b](facebook/react@f4f873f)
jerrydev0927
added a commit
to jerrydev0927/react
that referenced
this pull request
Jan 5, 2024
part of facebook/react#26571 merging separately to improve tracking of file renames DiffTrain build for [ffb8eaca5966fc7733bd0a23f4055e26d2cc59d7](facebook/react@ffb8eac)
EdisonVan
pushed a commit
to EdisonVan/react
that referenced
this pull request
Apr 15, 2024
First part of facebook#26571 merging separately to help with git history with a lot of file renames
EdisonVan
pushed a commit
to EdisonVan/react
that referenced
this pull request
Apr 15, 2024
…26590) Part of facebook#26571 Implements wiring for Flight to have it's own "HostConfig" from Fizz. Historically the ServerFormatConfigs were supposed to be generic enough to be used by Fizz and Flight. However with the addition of features like Float the configs have evolved to be more specific to the renderer. We may want to get back to a place where there is a pure FormatConfig which can be shared but for now we are embracing the fact that these runtimes need very different things and DCE cannot adequately remove the unused stuff for Fizz when pulling this dep into Flight so we are going to fork the configs and just maintain separate ones. At first the Flight config will be almost empty but once Float support in Flight lands it will have a more complex implementation Additionally this commit normalizes the component files which make up FlightServerConfig and FlightClientConfig. Now each file that participates starts with ReactFlightServerConfig... and ReactFlightClientConfig...
EdisonVan
pushed a commit
to EdisonVan/react
that referenced
this pull request
Apr 15, 2024
part of facebook#26571 merging separately to improve tracking of file renames
EdisonVan
pushed a commit
to EdisonVan/react
that referenced
this pull request
Apr 15, 2024
facebook#26592) part of facebook#26571 merging separately to improve tracking of files renames in git Rename HostConfig files to FiberConfig to clarify they are configs for Fiber and not Fizz/Flight. This better conforms to the naming used in Flight and now Fizz of `ReactFlightServerConfig` and `ReactFizzConfig`
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
ServerFormatConfigs no longer just provide generalizable Format configuration. The original intent was these files could be used by Fizz and Flight however the logic is deeply entangled with things like the ResponseState which are Fizz specific that at this point the file is not general at all.
Additionally we are about to have entirely different requirements for FlightServer for Float methods. We woudl need to extract Float from the format config to do this however there are performance advantages to having Float deeply integrated with what is called the format config.
This PR refactors config namings to lean into the fact that each runtime needs runtime specific configs and names them accordingly
Config for each runtime
Flight Server ->
ReactFlightServerConfigFlight Client ->
ReactFlightClientConfigFizz Server ->
ReactFizzConfig(Fiber) Client ->
ReactFiberConfigThis PR was set up as a series of commits focussing on FlightServer/FlightClient, then Fizz, and finally Fiber. It is probably easier to review commit by commit.
FlightServerConfigDOM currently simply exposes the
isPrimaryRendererflag. It will eventually hold the FlightServer implementation of FloatFlightClientConfigDOM currently holds nothing but will hold the FlightClient implementation of Float (in particular the machinery to dispatch directives from a Flight response in either Fizz or Fiber)