Skip to content

Fix RSC client loader support #13663

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

Merged
merged 9 commits into from
May 28, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 11 additions & 5 deletions packages/react-router/lib/dom/ssr/single-fetch.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -166,10 +166,13 @@ type GetRouteInfoFunction = (match: DataRouteMatch) => {
hasShouldRevalidate: boolean;
};

type ShouldAllowOptOutFunction = (match: DataRouteMatch) => boolean;

export type FetchAndDecodeFunction = (
args: DataStrategyFunctionArgs,
basename: string | undefined,
targetRoutes?: string[]
targetRoutes?: string[],
shouldAllowOptOut?: ShouldAllowOptOutFunction
) => Promise<{ status: number; data: DecodedSingleFetchResults }>;

export function getTurboStreamSingleFetchDataStrategy(
Expand Down Expand Up @@ -203,7 +206,8 @@ export function getSingleFetchDataStrategyImpl(
getRouteInfo: GetRouteInfoFunction,
fetchAndDecode: FetchAndDecodeFunction,
ssr: boolean,
basename: string | undefined
basename: string | undefined,
shouldAllowOptOut: ShouldAllowOptOutFunction = () => true
): DataStrategyFunction {
return async (args) => {
let { request, matches, fetcherKey } = args;
Expand Down Expand Up @@ -266,7 +270,8 @@ export function getSingleFetchDataStrategyImpl(
getRouteInfo,
fetchAndDecode,
ssr,
basename
basename,
shouldAllowOptOut
);
};
}
Expand Down Expand Up @@ -354,7 +359,8 @@ async function singleFetchLoaderNavigationStrategy(
getRouteInfo: GetRouteInfoFunction,
fetchAndDecode: FetchAndDecodeFunction,
ssr: boolean,
basename: string | undefined
basename: string | undefined,
shouldAllowOptOut: (match: DataRouteMatch) => boolean = () => true
) {
// Track which routes need a server load for use in a `_routes` param
let routesParams = new Set<string>();
Expand Down Expand Up @@ -396,7 +402,7 @@ async function singleFetchLoaderNavigationStrategy(

// When a route has a client loader, it opts out of the singular call and
// calls it's server loader via `serverLoader()` using a `?_routes` param
if (hasClientLoader) {
if (shouldAllowOptOut(m) && hasClientLoader) {
if (hasLoader) {
foundOptOutRoute = true;
}
Expand Down
47 changes: 35 additions & 12 deletions packages/react-router/lib/rsc/browser.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -271,7 +271,13 @@ export function getRSCSingleFetchDataStrategy(
// pass map into fetchAndDecode so it can add payloads
getFetchAndDecodeViaRSC(decode),
ssr,
basename
basename,
// If we don't have an element, we need to hit the server loader flow
// regardless of whether the client loader calls `serverLoader` or not,
// otherwise we'll have nothing to render.
// TODO: Do we need to account for API routes? Do we need a
// `match.hasComponent` flag?
(match) => match.route.element != null
);
return async (args) =>
args.unstable_runClientMiddleware(async () => {
Expand All @@ -288,17 +294,27 @@ export function getRSCSingleFetchDataStrategy(
context.set(renderedRoutesContext, []);
let results = await dataStrategy(args);
// patch into router from all payloads in map
const renderedRouteById = new Map(
context.get(renderedRoutesContext).map((r) => [r.id, r])
);
// TODO: Confirm that it's correct for us to have multiple rendered routes
// with the same ID. This is currently happening in `clientLoader` cases
// where we're calling `fetchAndDecode` multiple times. This may be a
// sign of a logical error in how we're handling client loader routes.
const renderedRoutesById = new Map<string, RenderedRoute[]>();
for (const route of context.get(renderedRoutesContext)) {
if (!renderedRoutesById.has(route.id)) {
renderedRoutesById.set(route.id, []);
}
renderedRoutesById.get(route.id)!.push(route);
}
for (const match of args.matches) {
const rendered = renderedRouteById.get(match.route.id);
if (rendered) {
window.__router.patchRoutes(
rendered.parentId ?? null,
[createRouteFromServerManifest(rendered)],
true
);
const renderedRoutes = renderedRoutesById.get(match.route.id);
if (renderedRoutes) {
for (const rendered of renderedRoutes) {
window.__router.patchRoutes(
rendered.parentId ?? null,
[createRouteFromServerManifest(rendered)],
true
);
}
}
}
return results;
Expand Down Expand Up @@ -457,7 +473,14 @@ function createRouteFromServerManifest(
let hasInitialError = payload?.errors && match.id in payload.errors;
let initialError = payload?.errors?.[match.id];
let isHydrationRequest =
match.clientLoader?.hydrate === true || !match.hasLoader;
match.clientLoader?.hydrate === true ||
!match.hasLoader ||
// If we don't have an element, we need to hit the server loader flow
// regardless of whether the client loader calls `serverLoader` or not,
// otherwise we'll have nothing to render.
// TODO: Do we need to account for API routes? Do we need a
// `match.hasComponent` flag?
!match.element;

let dataRoute: DataRouteObjectWithManifestInfo = {
id: match.id,
Expand Down