fix: generalize hydration cleanup #6118
Conversation
WalkthroughIntroduces framework-specific hydrateStart wrappers, externalizes SSR types, changes TSR lifecycle to explicit h/e/c signals, switches script cleanup to DOM removal via document.currentScript.remove(), and updates script emission and imports across router and start-client packages. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor App as Framework App
participant HS as hydrateStart (wrapper)
participant Core as start-client-core (core hydrateStart)
participant Router as Router (instance)
participant TSR as window.$_TSR
participant SSR as SSR stream
App->>HS: call hydrateStart()
HS->>Core: await core hydrateStart()
Core->>Router: initialize & hydrate
Core-->>HS: return Router
HS->>TSR: h() -- mark hydrated
HS-->>App: return Router
Note over SSR: streaming scripts continue emitting
SSR->>TSR: e() -- mark stream ended
TSR->>TSR: streamEnded = true
alt hydrated && streamEnded
TSR->>TSR: c() -- cleanup & delete window.$_TSR
end
SSR->>SSR: final script executes -> document.currentScript.remove()
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45–75 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used📓 Path-based instructions (2)**/*.{ts,tsx}📄 CodeRabbit inference engine (AGENTS.md)
Files:
**/*.{js,ts,tsx}📄 CodeRabbit inference engine (AGENTS.md)
Files:
🧠 Learnings (4)📓 Common learnings📚 Learning: 2025-10-08T08:11:47.088ZApplied to files:
📚 Learning: 2025-11-02T16:16:24.898ZApplied to files:
📚 Learning: 2025-10-01T18:30:26.591ZApplied to files:
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
🔇 Additional comments (3)
Comment |
|
View your CI Pipeline Execution ↗ for commit 5191c56
☁️ Nx Cloud last updated this comment at |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
packages/solid-start-client/src/hydrateStart.ts (1)
1-12: Duplicate implementation detected.This implementation is identical to the React version in
packages/react-start-client/src/hydrateStart.ts. Please refer to the review comment on that file regarding consolidating these identical wrappers.
🧹 Nitpick comments (4)
packages/react-start-client/src/hydrateStart.ts (2)
1-12: Consider consolidating identical framework wrappers.The React and Solid implementations of
hydrateStartare identical (seepackages/solid-start-client/src/hydrateStart.ts). This violates the DRY principle and increases maintenance burden.Consider extracting this wrapper logic into a shared utility in
@tanstack/start-client-corethat accepts the framework-specific hydration function, or document why framework-specific wrappers are architecturally necessary despite identical implementations.Based on learnings, separate framework-agnostic core logic from React/Solid bindings.
10-10: Add type augmentation for window.$_TSR.The access to
window.$_TSR?.h()lacks type safety. Consider adding a type declaration or augmentation to define the$_TSRproperty on the Window interface.Example type augmentation:
declare global { interface Window { $_TSR?: { h: () => void e: () => void } } }packages/router-core/src/ssr/types.ts (1)
21-40: Document the two-phase completion model in the interface comments.The interface introduces a sophisticated two-phase lifecycle with
h(),e(), andc(), but the comments are brief. Consider expanding the documentation to clarify:
- When
h()vse()should be called- That
c()only executes cleanup when both phases complete- The purpose of the
bufferandp()mechanismApply this diff to enhance documentation:
export interface TsrSsrGlobal { router?: DehydratedRouter - // Signal that router hydration is complete + /** + * Signal that router hydration is complete (phase 1). + * Must be paired with e() to trigger final cleanup. + */ h: () => void - // Signal that stream has ended + /** + * Signal that SSR stream has ended (phase 2). + * Must be paired with h() to trigger final cleanup. + */ e: () => void - // Cleanup all hydration resources and scripts + /** + * Cleanup all hydration resources and scripts. + * Only executes when both h() and e() have been called. + */ c: () => void - // p: Push script into buffer or execute immediately + /** + * Push script into buffer or execute immediately. + * Scripts are buffered until initialized=true, then executed. + */ p: (script: () => void) => voidpackages/router-core/src/ssr/tsrScript.ts (1)
2-15: Consider guarding against repeated lifecycle calls.The two-phase completion model is correctly implemented: cleanup only occurs when both
hydratedandstreamEndedare true. However, there are no guards against callingh()ore()multiple times, which could lead to unexpected behavior if lifecycle methods are invoked incorrectly.Consider adding guards to prevent repeated calls:
self.$_TSR = { h() { + if (this.hydrated) return this.hydrated = true this.c() }, e() { + if (this.streamEnded) return this.streamEnded = true this.c() },This makes the lifecycle more defensive and idempotent.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (17)
packages/react-router/src/ScriptOnce.tsx(1 hunks)packages/react-start-client/src/StartClient.tsx(1 hunks)packages/react-start-client/src/hydrateStart.ts(1 hunks)packages/react-start-client/src/index.tsx(1 hunks)packages/react-start-client/tsconfig.json(1 hunks)packages/router-core/src/ssr/ssr-client.ts(1 hunks)packages/router-core/src/ssr/ssr-server.ts(4 hunks)packages/router-core/src/ssr/tsrScript.ts(1 hunks)packages/router-core/src/ssr/types.ts(1 hunks)packages/solid-router/src/ScriptOnce.tsx(1 hunks)packages/solid-start-client/src/hydrateStart.ts(1 hunks)packages/solid-start-client/src/index.tsx(1 hunks)packages/start-client-core/src/client/index.ts(1 hunks)packages/start-client-core/src/index.tsx(1 hunks)packages/vue-router/src/ScriptOnce.tsx(1 hunks)packages/vue-router/src/Scripts.tsx(0 hunks)packages/vue-start-client/src/StartClient.tsx(1 hunks)
💤 Files with no reviewable changes (1)
- packages/vue-router/src/Scripts.tsx
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use TypeScript strict mode with extensive type safety for all code
Files:
packages/react-start-client/src/index.tsxpackages/router-core/src/ssr/types.tspackages/solid-start-client/src/hydrateStart.tspackages/react-start-client/src/StartClient.tsxpackages/vue-start-client/src/StartClient.tsxpackages/router-core/src/ssr/ssr-client.tspackages/start-client-core/src/index.tsxpackages/start-client-core/src/client/index.tspackages/solid-start-client/src/index.tsxpackages/react-router/src/ScriptOnce.tsxpackages/router-core/src/ssr/ssr-server.tspackages/solid-router/src/ScriptOnce.tsxpackages/vue-router/src/ScriptOnce.tsxpackages/router-core/src/ssr/tsrScript.tspackages/react-start-client/src/hydrateStart.ts
**/*.{js,ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Implement ESLint rules for router best practices using the ESLint plugin router
Files:
packages/react-start-client/src/index.tsxpackages/router-core/src/ssr/types.tspackages/solid-start-client/src/hydrateStart.tspackages/react-start-client/src/StartClient.tsxpackages/vue-start-client/src/StartClient.tsxpackages/router-core/src/ssr/ssr-client.tspackages/start-client-core/src/index.tsxpackages/start-client-core/src/client/index.tspackages/solid-start-client/src/index.tsxpackages/react-router/src/ScriptOnce.tsxpackages/router-core/src/ssr/ssr-server.tspackages/solid-router/src/ScriptOnce.tsxpackages/vue-router/src/ScriptOnce.tsxpackages/router-core/src/ssr/tsrScript.tspackages/react-start-client/src/hydrateStart.ts
🧠 Learnings (4)
📓 Common learnings
Learnt from: CR
Repo: TanStack/router PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-06T15:03:07.223Z
Learning: Separate framework-agnostic core logic from React/Solid bindings
📚 Learning: 2025-11-02T16:16:24.898Z
Learnt from: nlynzaad
Repo: TanStack/router PR: 5732
File: packages/start-client-core/src/client/hydrateStart.ts:6-9
Timestamp: 2025-11-02T16:16:24.898Z
Learning: In packages/start-client-core/src/client/hydrateStart.ts, the `import/no-duplicates` ESLint disable is necessary for imports from `#tanstack-router-entry` and `#tanstack-start-entry` because both aliases resolve to the same placeholder file (`fake-start-entry.js`) in package.json during static analysis, even though they resolve to different files at runtime.
Applied to files:
packages/react-start-client/src/index.tsxpackages/router-core/src/ssr/types.tspackages/react-start-client/tsconfig.jsonpackages/solid-start-client/src/hydrateStart.tspackages/react-start-client/src/StartClient.tsxpackages/vue-start-client/src/StartClient.tsxpackages/router-core/src/ssr/ssr-client.tspackages/start-client-core/src/index.tsxpackages/start-client-core/src/client/index.tspackages/solid-start-client/src/index.tsxpackages/router-core/src/ssr/ssr-server.tspackages/react-start-client/src/hydrateStart.ts
📚 Learning: 2025-10-08T08:11:47.088Z
Learnt from: nlynzaad
Repo: TanStack/router PR: 5402
File: packages/router-generator/tests/generator/no-formatted-route-tree/routeTree.nonnested.snapshot.ts:19-21
Timestamp: 2025-10-08T08:11:47.088Z
Learning: Test snapshot files in the router-generator tests directory (e.g., files matching the pattern `packages/router-generator/tests/generator/**/routeTree*.snapshot.ts` or `routeTree*.snapshot.js`) should not be modified or have issues flagged, as they are fixtures used to verify the generator's output and are intentionally preserved as-is.
Applied to files:
packages/router-core/src/ssr/ssr-client.ts
📚 Learning: 2025-10-01T18:30:26.591Z
Learnt from: schiller-manuel
Repo: TanStack/router PR: 5330
File: packages/router-core/src/router.ts:2231-2245
Timestamp: 2025-10-01T18:30:26.591Z
Learning: In `packages/router-core/src/router.ts`, the `resolveRedirect` method intentionally strips the router's origin from redirect URLs when they match (e.g., `https://foo.com/bar` → `/bar` for same-origin redirects) while preserving the full URL for cross-origin redirects. This logic should not be removed or simplified to use `location.publicHref` directly.
Applied to files:
packages/router-core/src/ssr/ssr-client.tspackages/start-client-core/src/index.tsx
🧬 Code graph analysis (3)
packages/solid-start-client/src/hydrateStart.ts (1)
packages/react-start-client/src/hydrateStart.ts (1)
hydrateStart(7-12)
packages/router-core/src/ssr/ssr-server.ts (1)
packages/router-core/src/ssr/constants.ts (1)
GLOBAL_TSR(1-1)
packages/react-start-client/src/hydrateStart.ts (3)
packages/solid-start-client/src/hydrateStart.ts (1)
hydrateStart(7-12)packages/solid-start-client/src/index.tsx (1)
hydrateStart(2-2)packages/start-client-core/src/client/index.ts (1)
hydrateStart(1-1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Preview
- GitHub Check: Test
🔇 Additional comments (18)
packages/vue-router/src/ScriptOnce.tsx (2)
24-41: LGTM: Client-side hydration logic is correct.The implementation properly handles the hydration mismatch pattern:
- Renders an empty script with
data-allow-mismatchbefore mounting- Returns null after mounting to clean up the placeholder
- Works in tandem with the server-side self-removing script
This aligns well with Vue's hydration model and the PR's objective of generalizing hydration cleanup.
15-22: Browser compatibility is strong for modern applications.document.currentScript is well established and works across many devices and browser versions since July 2015. It is not supported in Internet Explorer, but overall cross-browser compatibility score is 92, which is acceptable for most modern applications. If IE support is required, a polyfill would be needed.
The self-removal pattern is sound for SSR hydration. The concatenation approach is appropriate for internally-controlled script content within the framework, where props.children originates from framework logic rather than user input.
packages/vue-start-client/src/StartClient.tsx (1)
13-21: The code inpackages/vue-start-client/src/StartClient.tsxis correct and properly typed. Type definitions forwindow.$_TSRwith theh()method already exist in the codebase through theTsrSsrGlobalinterface exported from@tanstack/router-core/ssr/client, and the global Window interface augmentation has not been removed. No changes are needed.Likely an incorrect or invalid review comment.
packages/react-start-client/src/index.tsx (1)
3-3: LGTM!The addition of
hydrateStartto the public API surface aligns well with the introduction of the framework-specific wrapper and maintains consistency with the Solid package.packages/react-start-client/src/StartClient.tsx (1)
3-3: LGTM!The import change correctly references the new local wrapper, ensuring the hydration completion signal
window.$_TSR?.h()is properly invoked while preserving existing behavior.packages/solid-start-client/src/index.tsx (1)
2-2: LGTM!The export change maintains consistency with the React package structure and correctly exposes the Solid-specific hydration wrapper.
packages/react-start-client/tsconfig.json (1)
7-7: The removal of../start-client-core/src/startEntry.d.tsfrom react-start-client's tsconfig include does not affect type augmentation for this package. The module augmentation declared in that file is for the internal virtual modules#tanstack-start-entryand#tanstack-router-entry, which are used only by start-client-core's hydrateStart implementation, not by react-start-client. React-start-client properly receives types through the package's exported types, making the direct file inclusion unnecessary.Likely an incorrect or invalid review comment.
packages/solid-router/src/ScriptOnce.tsx (1)
18-18: LGTM! DOM-based script removal is cleaner.The change from invoking a global cleanup function to direct DOM removal via
document.currentScript.remove()simplifies the hydration lifecycle and eliminates the need for global coordination. This pattern is well-supported across modern browsers.packages/react-router/src/ScriptOnce.tsx (1)
16-16: LGTM! Consistent with the generalized cleanup approach.The switch to
document.currentScript.remove()is consistent with the Solid router implementation and correctly implements the new DOM-based cleanup pattern.packages/start-client-core/src/client/index.ts (1)
2-2: LGTM! Type-only export has no runtime impact.Adding the type re-export from
@tanstack/router-core/ssr/clientmakes SSR-related types publicly available without affecting the runtime bundle. This aligns well with the PR's goal of consolidating SSR type definitions.packages/router-core/src/ssr/types.ts (1)
4-12: LGTM! Compact serialization format is appropriate for SSR.The
DehydratedMatchinterface uses single-letter property names (e.g.,i,b,l) to minimize the serialized payload size, which is a good optimization for SSR. The type mappings toMakeRouteMatchproperties are clear and complete.packages/router-core/src/ssr/ssr-server.ts (4)
93-93: LGTM! Script self-removal aligns with framework changes.The change to append
document.currentScript.remove()in the script buffer correctly implements the DOM-based cleanup pattern, consistent with the updates toScriptOncecomponents in React and Solid routers.
145-145: LGTM! Simplified script generation.Removing the
class="$tsr"attribute from injected scripts simplifies the HTML output. The script identification is no longer needed since scripts now self-remove viadocument.currentScript.remove().
225-225: LGTM! Lifecycle method invocation is correct.Changing from directly setting
streamEnd=trueto calling.e()properly uses the new lifecycle API, which handles the two-phase cleanup coordination.
249-249: RemoveclassName: '$tsr'from line 249 or clarify its necessity for buffered scripts.The
$tsrclass on the buffered script tag (line 249) is used in the cleanup functiondocument.querySelectorAll(".\\$tsr").forEach(e => { e.remove() })to identify and remove temporary SSR scripts after hydration. If this cleanup mechanism is still required, the class attribute should be retained. However, if dynamic script injection at line 146 no longer relies on this cleanup pattern, verify whether the buffered script still needs this identifier. If not, remove it for consistency with dynamically injected scripts.packages/router-core/src/ssr/ssr-client.ts (2)
5-10: LGTM! Type consolidation improves maintainability.The updated imports correctly reference types from the centralized
./typesmodule instead of inline definitions, improving code organization and reducing duplication.
32-278: The framework-specifichydrateStartwrappers correctly invokewindow.$_TSR?.h()after hydration.Verified across all three framework implementations:
- React:
packages/react-start-client/src/hydrateStart.tscallswindow.$_TSR?.h()aftercoreHydrateStart()- Solid:
packages/solid-start-client/src/hydrateStart.tscallswindow.$_TSR?.h()after hydration- Vue:
packages/vue-start-client/src/StartClient.tsxcallswindow.$_TSR?.h()The core
hydrate()function inssr-client.tscorrectly does not callh()directly; instead, each framework wrapper handles the lifecycle signal after calling the core hydration logic.packages/start-client-core/src/index.tsx (1)
1-1: No action needed. TheDehydratedRoutertype was never exported from@tanstack/start-client-coreand is not part of its public API. It remains available for internal use within@tanstack/router-corethrough the./ssr/typesmodule. No breaking change is introduced by this line change.
schiller-manuel
force-pushed
the
generalized-hydration
branch
from
2a6a678 to
1db5ceb
Compare
Summary by CodeRabbit
Bug Fixes
Refactor
✏️ Tip: You can customize this high-level summary in your review settings.