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

Disable FOW for all ssr:false and tighten up spa mode logic #12894

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

Conversation

brophdawg11
Copy link
Contributor

@brophdawg11 brophdawg11 commented Jan 29, 2025

Fixes a few ssr/prerender bugs found during testing:

  • Lazy route discovery used to be disabled for SPA Mode only but should be disabled for all ssr: false apps so we don't make any assumptions about the static file server's ability to serve __manifest and deal with search params
  • We had some loose logic around spa mode detection with the introduction of prerender - this has been fixed up so that SPA mode is now properly detected as ssr:false and prerendeing the root route (/) and nothing else
    • ssr: false and a more advanced prerender config is for advanced use cases - this unlocks the ability to run loaders during the build as well as render down past the rot route for prerendered pages

Copy link

changeset-bot bot commented Jan 29, 2025

🦋 Changeset detected

Latest commit: f99370d

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

@@ -1402,7 +1401,7 @@ export const reactRouterVitePlugin: ReactRouterVitePlugin = () => {
let route = getRoute(ctx.reactRouterConfig, id);
if (!route) return;

if (!options?.ssr && !ctx.reactRouterConfig.ssr) {
if (!options?.ssr && isSpaModeEnabled(ctx.reactRouterConfig)) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Only reject server-only exports in true SPA mode

reactRouterConfig.prerender.length === 1 &&
reactRouterConfig.prerender[0] === "/"))
);
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is the proper detection, the old buggy one was simply:

!ctx.reactRouterConfig.ssr && ctx.reactRouterConfig.prerender == null

@@ -1908,13 +1920,6 @@ async function handlePrerender(
);
}
}

await prerenderManifest(
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Never need to prerender a manifest anymore

  • ssr: true will handle the manifest via the server handler
  • ssr: false will not have fog of war enabled

export function isFogOfWarEnabled(isSpaMode: boolean) {
return !isSpaMode;
export function isFogOfWarEnabled(ssr: boolean) {
return ssr === true;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Only enable fog of war for ssr:true apps

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.

1 participant