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

Build a version of server components that works in Vite and Workers runtimes #249

Open
jplhomer opened this issue Nov 16, 2021 · 6 comments
Assignees
Labels
blocked Can't continue until something else is fixed framework Related to framework aspects of Hydrogen
Milestone

Comments

@jplhomer
Copy link
Contributor

jplhomer commented Nov 16, 2021

This might be packaged into React core as a sibling package to react-server-dom-webpack, e.g. react-server-dom-vite. Alternatively, it might be a standalone plugin or package. It depends on what direction the core team decides and what other meta-frameworks do (like Next.js).

  • A Vite plugin that conditionally transforms client components into module references when the react-server flag is present.
  • A Vite plugin which creates a manifest of client components and passes it to the pipeToNodeWritable React function on the server, which generates the RSC response
  • A client runtime plugin which dynamically imports client components based on a payload from a RSC server response

Outstanding questions:

  • How do we get Vite to behave differently based on react-server context? React uses node --conditions react-server. Do we need two separate server builds or dev processes (this seems inefficient)? Is there a better way to do this?
  • Is the react-server condition going to work in a Workers runtime, where a single bundle is present and executed in a v8 isolate? How is this going to work?

Note: This is a bit of a moving target, as we've recently embarked on a mission to help React bundle client components more efficiently: facebook/react#22314

So whatever we build for Vite should match the output of those explorations.

@jplhomer jplhomer added the framework Related to framework aspects of Hydrogen label Nov 16, 2021
@morganmccunn morganmccunn added this to the v1.0.0 milestone Nov 17, 2021
@michenly
Copy link
Contributor

How do we get Vite to behave differently based on react-server context? React uses node --conditions react-server. Do we need two separate server builds or dev processes (this seems inefficient)? Is there a better way to do this?

There are two type of way node --conditions react-server flag is being use

  1. with https://github.com/facebook/react/blob/main/packages/react-server-dom-webpack/src/ReactFlightWebpackNodeLoader.js where there is an inline check in runtime
  2. with https://github.com/facebook/react/blob/main/packages/react-fs/index.js#L12 (and a few others), where it throws an error in default index file unless you load the correct node or browser package. (it is also listed in package.json 's export field)

I am not exactly sure what to do with 1, but for two I am curious to explore loading the correct module base on file post fix. ie. if the file importing it is .server.js, load the node version, if its .client.js, load the browser version. I think this can be done with some sort of Vite plugin.

@frandiox
Copy link
Contributor

frandiox commented Dec 1, 2021

So I've been tinkering with the offical RSC and found out it directly crashes when a Provider is used. Since our dev project is full of providers, I'm testing with a dummy app without any providers (we are supposed to remove them anyway). The whole tree looks like this (CartIcon is the only Client Component):

<div>
      <div>hello!</div>
      <CartIcon />
</div>

RSC response using our custom RSC implementation (in main branch):

M1:{"name":"CartIcon","id":"/Users/frandiox/src/github.com/Shopify/hydrogen/packages/dev/src/components/CartIcon.client.jsx","named":false}
J0:["$","div",null,{"children":[["$","div",null,{"key":0,"children":"hello!"}],["$","@1",null,{"children":null}]]}]

RSC response using the official implementation:

M1:{"id":"/Users/frandiox/src/github.com/Shopify/hydrogen/packages/dev/src/components/CartIcon.client.jsx","name":"CartIcon","named":false}
J0:["$","div",null,{"children":[["$","div",null,{"children":"hello!"}],["$","@1",null,{}]]}]

The only different is that the official approach is missing "key": 0.


However, when adding Suspense to the root of the dummy app, this happens.

RSC response using our custom RSC implementation (with Suspense):

M1:{"name":"CartIcon","id":"/Users/frandiox/src/github.com/Shopify/hydrogen/packages/dev/src/components/CartIcon.client.jsx","named":false}
J0:["$","div",null,{"key":1,"children":[["$","div",null,{"key":0,"children":"hello!"}],["$","@1",null,{"children":null}]]}]

RSC response using the official implementation:

S1:"react.suspense"
M2:{"id":"/Users/frandiox/src/github.com/Shopify/hydrogen/packages/dev/src/components/CartIcon.client.jsx","name":"CartIcon","named":false}
J0:["$","$1",null,{"children":["$","div",null,{"children":[["$","div",null,{"children":"hello!"}],["$","@2",null,{}]]}]}]

@wizardlyhel @jplhomer Do you think these differences are normal? Is the Suspense supposed to appear in the payload of the official implementation? 🤔

NOTE: this is still using our custom logic in the browser to parse the RSC response, that's why I'm passing "named" instead of "chunks".

@frandiox frandiox mentioned this issue Dec 1, 2021
4 tasks
@wizardlyhel
Copy link
Collaborator

@frandiox Yup - the S1:"react.suspense" is normal. Think of them are acting like client components waiting to be downloaded.

I'm gonna defer to @jplhomer on how he implemented Suspense with our custom implementation with key

@jplhomer
Copy link
Contributor Author

jplhomer commented Dec 2, 2021

@frandiox Yep you're spot-on. key is probably there because of my super hacky parsing mechanism 😬 we should trust whatever the official RSC implementation offers.

Additionally, the official RSC implementation uses name: <name> rather than named: true/false, and that matches how RSC resolves everything in the client. (this is before I learned how all of this worked)

@frandiox frandiox added the blocked Can't continue until something else is fixed label Dec 6, 2021
@frandiox
Copy link
Contributor

frandiox commented Dec 6, 2021

Blocked by:

  • Hydrogen still uses Provider/Context everywhere.
  • ReadableStream not yet supported in workers runtime.
  • @shopify/react-testing doesn't work with React 18 experimental. (fixed in chore: upgrade to latest React 18 experimental version #312 )
  • Unpublished react-server and react-client packages (they won't be published).

@frandiox
Copy link
Contributor

Update: Since react-server and react-client packages won't be published to NPM, we need to make a PR to React repo instead: facebook/react#22952

rafaelstz pushed a commit to rafaelstz/hydrogen that referenced this issue Mar 4, 2023
Fix multiple option variants and available filter
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blocked Can't continue until something else is fixed framework Related to framework aspects of Hydrogen
Projects
None yet
Development

No branches or pull requests

5 participants