-
Notifications
You must be signed in to change notification settings - Fork 324
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
Official RSC exploration #317
Conversation
const cache: ReturnType<typeof createResponseCache> = | ||
unstable_getCacheForType(createResponseCache); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice to know that we can get unstable_getCacheForType
working here. I had a lot of trouble to get by that --condition react-server
flag with the webpack version.
But since we are looking for a Vite implementation package that will include react-server
stuff, do we still need to put this functionality behind some kinda flag? (Asking without in-depth knowledge around Vite plugin ecosystem)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If this approach or something similar is "approved" by the React team, then we won't need any flag since there's only 1 environment running. I'm not sure if they will like the fact that we "merge" components and modules in the same environment, though 😬
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That is what react-server-dom-webpack
did. It includes codes from react-server
.. and I believe that is how React wants to move forward with any framework integration with react-server
.
We can double check that in the working group.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@wizardlyhel Yes, they use Rollup to combine different files from webpack and react-server
. However, they also generate a "standalone" version of react-server that can just be imported (and pass all the webpack/vite overrides as parameters). This is what I'm using in this PR.
However, when making a PR to React repo, I'll change it to the other approach using Rollup (they said they are not going to publish react-server
and react-client
packages).
What I meant with "merge" components is that in this PR, a client component is a proxy that returns different stuff depending on the current renderer (ssr / rsc) but it works with just 1 bundle and in 1 process. What I've seen in the demos and diagrams from the React team, however, is that you need either 2 bundles or at least different 2 Node environments.
moduleReference: ModuleReference<T> | ||
): ModuleMetaData { | ||
return { | ||
id: moduleReference.filepath, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I notice this filepath
is now the full filesystem path like "id":"/Users/ ... /Shopify/hydrogen/packages/dev/src/RSCTest/C1.client.jsx"
Should this be relative path to the serving root?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ideally yes but this is also connected to Vite's dynamic imports so we need to change that first (and might be troublesome).
Overall - I'm excited that this PR took out a lot of our custom logics. With regards to context/provider, this is work-in-progress of us removing packages that heavily depends on context like |
Tried to load this in worker and it didn't work. The RSC didn't even generate a serialized response. If you want to load the
|
@wizardlyhel Ah yes, this PR is not prepared for workers yet due to lack of ReadableStream. As you know, in the previous approach we were buffering the RSC response and returning plain text instead of streaming in workers. However, I haven't implemented that buffering here since this is still blocked by other stuff and the ReadableStream should be available soon. If you are curious, I think the RSC request in workers is dying either here or here. |
This PR is 🤯. Maybe I'll schedule a time where @frandiox you could could walk me through it? |
Right - I keep forgetting about the missing ReadableStream support in workers. |
@blittle Sure! Tuesday/Wednesday morning JST (your afternoon/evening 1 day before) works for me 👍 |
Combine SSR and RSC responses + Request Provider
fix: fix tests with normalizing paths
@Aslemammad @blittle it's finally greeeeeen 🟢 💚 🍏 📗 |
@frandiox Thank you for the opportunity; that's the least I could do! |
Update: I added back I also removed This unlocks:
I think it makes sense for I feel good about merging this into an experimental branch. |
* feat: implement RSC using react-server and react-client (#317) * wip: Create custom flight renderer with local version of react-server * wip: mock client manifest and add module reference to client components * wip: hack RSC resolution * wip: wrap component in Proxy to access original properties * refactor: rename RSC server files * wip: add official RSC hydrator * fix: Rename response.readRoot and remove explicit hydration * wip: remove RR and Helmet providers from the server * wip: Add test app * wip: remove hydration providers * wip: add renderToReadableStream for RSC * refactor: cleanup custom RSC code * refactor: move and rename files * wip: fix test app * refactor: cleanup custom RSC code * refactor: do not pass named boolean in RSC * refactor: simplify ClientMarker * test: fix ClientMarker tests * fix: delay throwing error when client component is missing * refactor: simplify import globs code * feat: provide request object to rendering tree * wip: example of useServerRequest * feat: inline RSC response in the SSR HTML response * fix: add default value to the SSR provider * feat: stream rsc in script tags * feat: update the starter template to work * fix: lint errors * refactor: move code around and cleanup * chore: upgrade Vite to 2.7.0 * chore: add react-server-dom-vite as vendor * feat: replace local react-server and react-client with react-server-dom-vite vendor * fix: react-server-dom-vite allow exporting hooks from client components temporarily * fix: move the server logging locations * feat: Implement ReadableStream branch in SSR and RSC * test: fix playground apps * fix: avoid importing undefined exports depending on the running environment * feat: Buffer RSC response if it cannot be streamed * fix: tests * fix: e2e test * fix: linting * fix: rename test helper * fix: update hydrogen template files * feat: remove react-ssr-prepass. RSC already makes ReadableStream required * refactor: simplify hydration calls * feat: remove old dependencies * feat: enable tree shaking in worker build * fix: Release stream lock before piping * chore: fix formatting * refactor: move code to entry-server to be consistent * refactor: remove HydrationWriter in favor of Node native PassThrough * fix: move constant to a separate file to avoid importing app logic in GraphiQL * refactor: remove old code * fix: maybe fix tests for windows * fix: stream import in workers * fix: flush RSC right after writing head * feat: replace RenderCacheProvider with new ServerRequestProvider cache * refactor: cleanup * fix: suspense breaking hydration * fix: normalize RSC chunks * refactor: simplify customBody check * feat: minor tree-shaking improvements * fix: enable browser hydration * fix: replace TransformStream with ReadableStream to support Firefox * refactor: cleanup and rename variables * fix: normalizePath * fix: better regex * fix: request.context property is reserved in CFW * perf: Do not clone request to avoid extra memory and warnings in CFW * chore: add TODO to remove weird Suspense boundary * fix: Add back ShopifyProvider; call setShopConfig as part of renderHydrogen * chore: remove RSCTest demo code * fix: useMemo is allowed in RSC * fix: Dot not create Response with streams to support CFW Co-authored-by: Bret Little <bret.little@shopify.com> Co-authored-by: M. Bagher Abiat <zorofight94@gmail.com> Co-authored-by: Josh Larson <josh.larson@shopify.com> * feat: Consume RSC response in the server (#463) * wip: Create custom flight renderer with local version of react-server * wip: mock client manifest and add module reference to client components * wip: hack RSC resolution * wip: wrap component in Proxy to access original properties * refactor: rename RSC server files * wip: add official RSC hydrator * fix: Rename response.readRoot and remove explicit hydration * wip: remove RR and Helmet providers from the server * wip: Add test app * wip: remove hydration providers * wip: add renderToReadableStream for RSC * refactor: cleanup custom RSC code * refactor: move and rename files * wip: fix test app * refactor: cleanup custom RSC code * refactor: do not pass named boolean in RSC * refactor: simplify ClientMarker * test: fix ClientMarker tests * fix: delay throwing error when client component is missing * refactor: simplify import globs code * feat: provide request object to rendering tree * wip: example of useServerRequest * feat: inline RSC response in the SSR HTML response * fix: add default value to the SSR provider * feat: stream rsc in script tags * feat: update the starter template to work * fix: lint errors * refactor: move code around and cleanup * chore: upgrade Vite to 2.7.0 * chore: add react-server-dom-vite as vendor * feat: replace local react-server and react-client with react-server-dom-vite vendor * fix: react-server-dom-vite allow exporting hooks from client components temporarily * fix: move the server logging locations * feat: Implement ReadableStream branch in SSR and RSC * test: fix playground apps * fix: avoid importing undefined exports depending on the running environment * feat: Buffer RSC response if it cannot be streamed * fix: tests * fix: e2e test * fix: linting * fix: rename test helper * fix: update hydrogen template files * feat: remove react-ssr-prepass. RSC already makes ReadableStream required * refactor: simplify hydration calls * feat: remove old dependencies * feat: enable tree shaking in worker build * fix: Release stream lock before piping * chore: fix formatting * refactor: move code to entry-server to be consistent * refactor: remove HydrationWriter in favor of Node native PassThrough * fix: move constant to a separate file to avoid importing app logic in GraphiQL * refactor: remove old code * fix: maybe fix tests for windows * fix: stream import in workers * fix: flush RSC right after writing head * feat: replace RenderCacheProvider with new ServerRequestProvider cache * refactor: cleanup * fix: suspense breaking hydration * fix: normalize RSC chunks * refactor: simplify customBody check * feat: minor tree-shaking improvements * fix: enable browser hydration * fix: replace TransformStream with ReadableStream to support Firefox * refactor: cleanup and rename variables * feat: Make ReadableStream globally available * wip: consume RSC response in server * wip: do not close stream connection * wip: fix reentrant error with ReadableStreams in RSC * fix: import Node RSC logic dynamically * fix: rename Cache.client.js to avoid bundling it in workers build * fix: add temporary monkey patch for React Fizz in workers build * feat: Combine SSR and RSC responses in Node * fix: write RSC before ending response when not streaming * feat: Combine SSR and RSC responses in Worker * feat: Use ReadableStream in CFW and stream if supported * fix: add another temporary monkey patch for React Fizz/Flight in workers build * fix: close writable on redirects * fix: Fix hydration during dev by using more Suspense boundaries * fix: add back response.socket listener * feat: support script nonce and shorten access to __flight * feat: avoid writing fractional chunks in SSR * fix: initialize flight container in buffered rendering * fix: ensure flight container is added in Node * refactor: `readable` to `ssrReadable` for clarity Co-authored-by: Bret Little <bret.little@shopify.com> Co-authored-by: Josh Larson <josh.larson@shopify.com> * fix: make ServerHandlerConfig required * chore: update changelog * chore: update docs Co-authored-by: Fran Dios <frankdiox@gmail.com> Co-authored-by: Bret Little <bret.little@shopify.com> Co-authored-by: M. Bagher Abiat <zorofight94@gmail.com>
Description
Related #249
Background
Incompatibilities with official React 18 experimental
As far as I understand, React 18 experimental supports the following features in isolation:
--conditions react-server
to the Node runtime.It is not obvious to me that both features can be combined right now, because based on the public demos you either load actual client components or only the descriptors in the server.
However, Hydrogen needs both, components and descriptors, at the same time in order to do SSR + RSC in the same process. This could be achieved by importing 2 different server bundles but this seems problematic and would slow down the development server.
On top of that, Hydrogen is supposed to run on Oxygen/workers, where nothing like
--conditions
can be passed to the runtime.And of course, Hydrogen uses Vite instead of Webpack, which is the only tool that has official support at the moment.
New approach
Instead of Node
--conditions
, this PR uses a hook in Vite to generate the descriptor. This descriptor is "merged" with the actual component in a proxy and will behave differently depending on the renderer (we don't need Provider/Context anymore).When the app tree is passed to React SSR renderer, the proxied client components will be returned as normal components -- they are actually wrapped in React forward refs due to some constraints but the point is that they are rendered normally as real components.
However, when the app tree is passed to React RSC renderer, and thanks to a customizable
isModuleReference
hook, the proxied client components will behave as RSC descriptor instead, outputting the correct wire/flight syntax.Apart from that, since we are still using Vite's dynamic imports from the client, we don't need a manifest to generate or interpret the RSC payload. We just pass the module ID to Vite's helper in the browser and it figures out the way to download it.
The tricky part in all of this is merging actual components with their descriptors so they work for both flows in the same process.
Also, the way this PR accesses React flight logic is by importing directly fromreact-server
andreact-client
packages, which are not published to NPM at the moment (they are linked to a local version of React).Unknowns
Arereact-server
andreact-client
going to be published to NPM, or do we need to make a PR to React repo with a Vite plugin instead?How does this work withhydrateRoot
? Related Library Upgrade Guide: <script> (e.g. SSR frameworks) reactwg/react-18#114How do we generate the RSC wire/flight payload and SSR at the same time? Right now we do SSR first and then RSC after that, which is obviously slower because it traverses the app tree twice (although fetch sub-requests should be cached and reused). We can make some tricks to preload RSC components in the frontend during SSR streaming but we still need to wait for the final RSC wire/flight payload for the actual hydration.Update
react-server
andreact-client
packages are not going to be published to NPM. We have now submitted a Vite plugin in React repo. A compiled version of this plugin has been added to this PR invendor/react-dom-server-vite
.Setup
yarn dev-lib
in Hydrogen rootyarn dev-server
in Hydrogen root when the lib finishesBefore submitting the PR, please make sure you do the following:
Unreleased
heading in the package'sCHANGELOG.md
fixes #123
)