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

feat: prep data-strategy for RSC re-use #13304

Open
wants to merge 1 commit into
base: dev
Choose a base branch
from
Open

Conversation

jacob-ebey
Copy link
Member

No description provided.

Copy link

changeset-bot bot commented Mar 25, 2025

🦋 Changeset detected

Latest commit: 6058a75

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 11 packages
Name Type
@react-router/dev Patch
react-router Patch
@react-router/fs-routes Patch
@react-router/remix-routes-option-adapter Patch
@react-router/architect Patch
@react-router/cloudflare Patch
react-router-dom Patch
@react-router/express Patch
@react-router/node Patch
@react-router/serve Patch
create-react-router Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

Comment on lines +152 to +154
getShouldRevalidate: (routeId) => routeModules[routeId]?.shouldRevalidate,
hasClientLoader: (routeId) => !!manifest.routes[routeId]?.hasClientLoader,
hasLoader: (routeId) => !!manifest.routes[routeId]?.hasLoader,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you think we could collapse these into a single getRouteInfo function?

Comment on lines +156 to +158
unwrapSingleFetchResults: (decoded, routeId) => {
return unwrapSingleFetchTurboStreamResults(decoded, routeId);
},
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It bugs me we need to separate fetchAndDecode from unwrap but based on the way it's currently written we don't have a choice. I'm going to take a quick peek at a small refactor to alleviate the need to split those...

// single fetch .data request
foundOptOutRoute = true;
}
foundOptOutRoute = true;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is causing the failing E2E test - we need to short circuit either way, and then want to mark foundOptOutRoute only when hasLoader()=true

if (m.route.id in router.state.loaderData && getShouldRevalidate(m.route.id)) {
  if (hasLoader(m.route.id)) {
    // If we have a server loader, make sure we don't include it in the
    // single fetch .data request
    foundOptOutRoute = true;
  }
  return
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants