-
Notifications
You must be signed in to change notification settings - Fork 325
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
WIP: RSC sub route #1858
base: v1.x-2022-07
Are you sure you want to change the base?
WIP: RSC sub route #1858
Conversation
@frandiox This branch is demonstrating how I think we can do an opted-in RSC sub route implementation without making it a major breaking change. To play with this, just uncomment out I notice that we virtually imports I am thinking to dynamically register these sub routes as SSR render discovers them .. that is if we can guarantee to have a SSR render before an RSC request. Another way is to have these sub routes to be also defined in the Thoughts? |
@frandiox I updated this PR to demonstrate sub routes working. I think this solution is quite elegant - it doesn't make a major breaking change. How it worksI put the sub route inside demo store's Thinking about how to get it not make a second
|
Thanks for putting this together! I haven't had much time yet to play with it but, if I didn't understand it wrong, I think there are a few challenges here:
Any idea how to deal with any of this? 🤔 A possible approach to explore for the first challenge would be changing
I've no idea how this would affect CPU performance though. |
If we send to the client the separate layout maps, then the client should know during transitions which RSC request to make and which not to, so we wouldn't need to rely on browser cache. |
WIP Sub route API proposalAll new function naming are not final How to opt-in using Hydrogen sub route
Global sub routesDefine global sub routes in export const HeaderRSCOutlet = defineRSCOutlet({
outletName: 'HeaderRSCOutlet', // haven't figure out how I can avoid the need to have this string to match the export name
component: Header,
});
function App({request}: HydrogenRouteProps) {
return (
<HeaderRSCOutlet />
);
}
export default renderHydrogen(App, {
HeaderRSCOutlet,
}); Page sub routesDefine page level sub routes in page routes, for example, const Test = ({id}: {id: string}) => (<p>Test RSC Outlet {id}</p>)
export const TestRSCOutlet = defineRSCOutlet({
outletName: 'TestRSCOutlet',
component: Test,
dependency: ['id'],
});
export default function Homepage() {
return (
<TestRSCOutlet id="123"/>
);
} |
if (serverProps.outlet && component) { | ||
console.log('RSCSub', serverProps.outlet, serverProps.response.cache); | ||
serverProps.response.cache(cache); | ||
return component ? component(state) : null; | ||
} else if (isRSC) { | ||
console.log('RSCSub - RSC'); | ||
return ( | ||
<RSCSubRouteClient outletName={outletName} state={state} isRSC={isRSC} /> | ||
); | ||
} else { | ||
console.log('RSCSub - SSR'); | ||
return ( | ||
<RSCSubRouteClient outletName={outletName} state={state} isRSC={isRSC}> | ||
{component(serverProps)} | ||
</RSCSubRouteClient> | ||
); |
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.
@frandiox I want to gather on your thoughts on this
So I have been noodling on this pattern for the outlets and I am wondering if it is do-able for the full page SSR as well.
What this code piece is doing ...
Say we have a server component
<Product handle={handle} /> => <p>Product title</p>
On SSR pass for /product/123
, this will render the SSR output of Product
and a RSCSubRouteClient
. We don't need a RSC payload to hydrate because the children of RSCSubRouteClient
is the exact content we need.
<RSCSubRouteClient state={{handle}}>
<p>Product title</p>
</RSCSubRouteClient>
On RSC pass for /product/123
, this will just render RSCSubRouteClient
with the state
<RSCSubRouteClient state={{handle}} />
The content of this route <p>Product title</p>
will be replaced by rscResponse.readRoot()
Do you think it is possible to do the same for the full page SSR render and we can drop the 2nd RSC pass entirely?
Description
RSC Sub routes - Allow Hydrogen to define sub routes with RSC:
Expected behaviour:
__rsc
call (for now)Before submitting the PR, please make sure you do the following:
fixes #123
)yarn changeset add
if this PR cause a version bump based on Keep a Changelog and adheres to Semantic Versioning