fix(start): handle Cloudflare SPA fallback hydration mismatch #6481
Conversation
📝 WalkthroughWalkthroughAdds SPA-shell detection and a global Changes
Sequence DiagramsequenceDiagram
actor User
participant Browser as Browser Window
participant Router as Router
participant Hydration as Hydration System
User->>Browser: Navigate to /some/path
Browser->>Browser: Read window.__TSS_SPA_SHELL__
alt SPA shell marker present
Browser->>Router: Check for router matches
Router-->>Browser: No matches
Browser->>Browser: Skip SSR hydration (client mount only)
else No SPA marker OR path == basepath OR matches exist
Browser->>Hydration: Perform SSR hydration
Hydration-->>Browser: Hydrated
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@packages/start-client-core/src/client/hydrateStart.ts`:
- Around line 45-55: The pathname equality check in hydrateStart.ts can fail
when routes differ only by a trailing slash; update the condition that compares
window.location.pathname to router.options.basepath to use the router path
normalization utilities (e.g., call removeTrailingSlash or use exactPathTest
from path.ts) so "/app" and "/app/" are treated as equal, and ensure you
reference window.__TSS_SPA_SHELL__, router.options.basepath and hydrate(router)
when making the change; also add a unit/integration test that asserts the logic
skips hydration for window.location.pathname = "/app/" when
router.options.basepath = "/app" to prevent regressions.
🧹 Nitpick comments (3)
packages/start-plugin-core/src/prerender.ts (1)
239-259: SPA shell marker injection logic is sound overall, but the fallback placement is outside<body>.The
</head>injection is correct. However, the fallback on lines 250-256 places the script before the<bodytag (between</head>and<body>), not "at the beginning of body" as the comment states. While browsers tolerate this, for semantic correctness the script should be injected inside the body element.Additionally, using a regex for the
<body>fallback would be more robust for matching variants like<body class="...">.♻️ Optional: Inject inside body element with regex matching
} else { - // Fallback: inject at the beginning of body if </head> not found - const bodyOpenTag = '<body' - if (html.includes(bodyOpenTag)) { - html = html.replace( - bodyOpenTag, - '<script>window.__TSS_SPA_SHELL__ = true</script>' + - bodyOpenTag, - ) - } + // Fallback: inject at the beginning of body if </head> not found + html = html.replace( + /<body[^>]*>/i, + (match) => match + '<script>window.__TSS_SPA_SHELL__ = true</script>', + ) }packages/start-client-core/tests/hydrateStart.test.ts (2)
47-59: Redundant initialization check in beforeEach.Lines 50-52 check
if (!mockGetRouter)before initializing, but the module-level mocks on lines 27-45 already use a getter with lazy initialization. This check is never true at this point since the mocks ensuremockGetRouteris initialized on first access.♻️ Remove redundant check
beforeEach(() => { - // Ensure mockGetRouter is initialized - if (!mockGetRouter) { - mockGetRouter = vi.fn(() => Promise.resolve(createMockRouter())) - } - vi.clearAllMocks() delete window.__TSS_SPA_SHELL__ delete window.__TSS_START_OPTIONS__ window.history.pushState({}, '', '/') mockGetRouter.mockResolvedValue(createMockRouter()) })
100-136: Consider adding a test for basepath trailing slash mismatch.The custom basepath tests cover exact matches and deeper paths, but don't cover the edge case where basepath and pathname differ only by a trailing slash (e.g., basepath
/appvs pathname/app/). This aligns with the potential issue in the implementation's strict equality comparison.📋 Suggested additional test case
it('should handle trailing slash mismatch between basepath and pathname', async () => { window.__TSS_SPA_SHELL__ = true window.history.pushState({}, '', '/app/') // trailing slash mockGetRouter.mockResolvedValueOnce(createMockRouter({ options: { basepath: '/app' } // no trailing slash })) await hydrateStart() // Current behavior: these don't match, so hydration is skipped // If trailing slash normalization is added, this should hydrate expect(hydrate).not.toHaveBeenCalled() })
…PA shell fallback detection.
|
why do we need this? what exactly is mismatching here? we should already handle this correctly in router IMO |
2 participants
Problem
When deploying a TanStack Start application to Cloudflare Pages with SPA mode enabled, directly navigating to non-root routes (e.g.,
/example) causes React hydration errors.Root Cause:
index.html(app shell) for all routes/example)Reproduction:
See Issue Hydration error on direct navigation to non-root routes in SPA mode #6455 and https://github.com/bbertold/tanstack-start-spa
Solution
Implemented a two-part fix:
1. Build-time Marker Injection (
start-plugin-core)window.__TSS_SPA_SHELL__ = trueinto the SPA shell during prerenderingspa.prerender.outputPath)</head>first, then<body>if not found2. Client-side Detection (
start-client-core)__TSS_SPA_SHELL__marker before hydrationThis fix is server-agnostic and will resolve similar hydration issues on servers like Caddy, Nginx, or Vercel when configured for SPA fallback 😄 👍
Testing
Unit Tests (11 new tests)
All tests passing ✅
Summary by CodeRabbit
New Features
Tests
✏️ Tip: You can customize this high-level summary in your review settings.