fix: route HMR handling #5710
Merged
fix: route HMR handling #5710
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
View your CI Pipeline Execution ↗ for commit d35c4c3
☁️ Nx Cloud last updated this comment at |
Contributor
WalkthroughThe changes remove the Changes
Sequence Diagram(s)sequenceDiagram
participant Vite as Vite HMR
participant OldModule as Old Route Module
participant NewModule as New Route Module
participant Router as Router Instance
participant Caches as Router Caches
Vite->>NewModule: Hot update received
NewModule->>Router: Check Route & newModule.Route
alt Route exists
NewModule->>NewModule: handleRouteUpdate(oldRoute, newRoute)
Note over NewModule: Copy private fields<br/>(_path, _id, _fullPath, _to, etc)
NewModule->>Caches: Update routesById[id]
NewModule->>Caches: Update routesByPath[path]
NewModule->>Caches: Replace in flatRoutes
NewModule->>Router: invalidate(filter: oldRouteId)
Router->>Router: Re-evaluate route tree
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
More templates
@tanstack/arktype-adapter
@tanstack/directive-functions-plugin
@tanstack/eslint-plugin-router
@tanstack/history
@tanstack/nitro-v2-vite-plugin
@tanstack/react-router
@tanstack/react-router-devtools
@tanstack/react-router-ssr-query
@tanstack/react-start
@tanstack/react-start-client
@tanstack/react-start-server
@tanstack/router-cli
@tanstack/router-core
@tanstack/router-devtools
@tanstack/router-devtools-core
@tanstack/router-generator
@tanstack/router-plugin
@tanstack/router-ssr-query-core
@tanstack/router-utils
@tanstack/router-vite-plugin
@tanstack/server-functions-plugin
@tanstack/solid-router
@tanstack/solid-router-devtools
@tanstack/solid-router-ssr-query
@tanstack/solid-start
@tanstack/solid-start-client
@tanstack/solid-start-server
@tanstack/start-client-core
@tanstack/start-plugin-core
@tanstack/start-server-core
@tanstack/start-static-server-functions
@tanstack/start-storage-context
@tanstack/valibot-adapter
@tanstack/virtual-file-routes
@tanstack/zod-adapter
commit: |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
packages/router-core/src/route.ts(0 hunks)packages/router-plugin/src/core/code-splitter/compilers.ts(3 hunks)packages/router-plugin/src/core/route-hmr-statement.ts(1 hunks)packages/router-plugin/tests/add-hmr/snapshots/react/[email protected](1 hunks)packages/router-plugin/tests/add-hmr/snapshots/react/[email protected](1 hunks)packages/router-plugin/tests/add-hmr/snapshots/solid/[email protected](1 hunks)
💤 Files with no reviewable changes (1)
- packages/router-core/src/route.ts
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use TypeScript in strict mode with extensive type safety across the codebase
Files:
packages/router-plugin/tests/add-hmr/snapshots/solid/[email protected]packages/router-plugin/src/core/code-splitter/compilers.tspackages/router-plugin/tests/add-hmr/snapshots/react/[email protected]packages/router-plugin/tests/add-hmr/snapshots/react/[email protected]packages/router-plugin/src/core/route-hmr-statement.ts
packages/{router-cli,router-generator,router-plugin,virtual-file-routes}/**
📄 CodeRabbit inference engine (AGENTS.md)
Keep CLI, generators, bundler plugins, and virtual file routing utilities in their dedicated tooling package directories
Files:
packages/router-plugin/tests/add-hmr/snapshots/solid/[email protected]packages/router-plugin/src/core/code-splitter/compilers.tspackages/router-plugin/tests/add-hmr/snapshots/react/[email protected]packages/router-plugin/tests/add-hmr/snapshots/react/[email protected]packages/router-plugin/src/core/route-hmr-statement.ts
packages/router-plugin/**
📄 CodeRabbit inference engine (AGENTS.md)
Use unplugin for universal bundler plugins in the router-plugin package
Files:
packages/router-plugin/tests/add-hmr/snapshots/solid/[email protected]packages/router-plugin/src/core/code-splitter/compilers.tspackages/router-plugin/tests/add-hmr/snapshots/react/[email protected]packages/router-plugin/tests/add-hmr/snapshots/react/[email protected]packages/router-plugin/src/core/route-hmr-statement.ts
🧠 Learnings (5)
📚 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-plugin/tests/add-hmr/snapshots/solid/[email protected]packages/router-plugin/src/core/code-splitter/compilers.tspackages/router-plugin/tests/add-hmr/snapshots/react/[email protected]packages/router-plugin/tests/add-hmr/snapshots/react/[email protected]packages/router-plugin/src/core/route-hmr-statement.ts
📚 Learning: 2025-09-23T17:36:12.598Z
Learnt from: CR
Repo: TanStack/router PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-09-23T17:36:12.598Z
Learning: Applies to packages/{react-router,solid-router}/** : Implement React and Solid bindings/components only in packages/react-router/ and packages/solid-router/
Applied to files:
packages/router-plugin/tests/add-hmr/snapshots/solid/[email protected]packages/router-plugin/tests/add-hmr/snapshots/react/[email protected]packages/router-plugin/tests/add-hmr/snapshots/react/[email protected]packages/router-plugin/src/core/route-hmr-statement.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-plugin/tests/add-hmr/snapshots/solid/[email protected]packages/router-plugin/src/core/route-hmr-statement.ts
📚 Learning: 2025-09-23T17:36:12.598Z
Learnt from: CR
Repo: TanStack/router PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-09-23T17:36:12.598Z
Learning: Applies to packages/router-core/** : Keep framework-agnostic core router logic in packages/router-core/
Applied to files:
packages/router-plugin/tests/add-hmr/snapshots/solid/[email protected]packages/router-plugin/tests/add-hmr/snapshots/react/[email protected]packages/router-plugin/tests/add-hmr/snapshots/react/[email protected]packages/router-plugin/src/core/route-hmr-statement.ts
📚 Learning: 2025-09-23T17:36:12.598Z
Learnt from: CR
Repo: TanStack/router PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-09-23T17:36:12.598Z
Learning: Applies to packages/router-plugin/** : Use unplugin for universal bundler plugins in the router-plugin package
Applied to files:
packages/router-plugin/tests/add-hmr/snapshots/react/[email protected]
🧬 Code graph analysis (5)
packages/router-plugin/tests/add-hmr/snapshots/solid/[email protected] (1)
packages/router-core/src/route.ts (1)
Route(586-776)
packages/router-plugin/src/core/code-splitter/compilers.ts (1)
packages/router-plugin/src/core/route-hmr-statement.ts (1)
routeHmrStatement(32-44)
packages/router-plugin/tests/add-hmr/snapshots/react/[email protected] (1)
packages/router-core/src/route.ts (1)
Route(586-776)
packages/router-plugin/tests/add-hmr/snapshots/react/[email protected] (1)
packages/router-core/src/route.ts (1)
Route(586-776)
packages/router-plugin/src/core/route-hmr-statement.ts (1)
packages/router-core/src/route.ts (1)
AnyRoute(778-797)
⏰ 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)
- GitHub Check: Test
Comment on lines
+19
to
+30
| newRoute.children = oldRoute.children | ||
| newRoute.parentRoute = oldRoute.parentRoute | ||
|
|
||
| const router = window.__TSR_ROUTER__! | ||
| router.routesById[newRoute.id] = newRoute | ||
| router.routesByPath[newRoute.fullPath] = newRoute | ||
| const oldRouteIndex = router.flatRoutes.indexOf(oldRoute) | ||
| if (oldRouteIndex > -1) { | ||
| router.flatRoutes[oldRouteIndex] = newRoute | ||
| } | ||
| router.invalidate({ filter: (m) => m.routeId === oldRoute.id }) | ||
| } |
Contributor
There was a problem hiding this comment.
Update parent/child bindings when swapping the route instance.
We copy private fields and replace the cache maps, but the parent route’s children array still holds oldRoute, and every child keeps its parentRoute pointing to the stale instance. Active matches and tree traversal walk those references, so beforeLoad/loader calls continue to use the pre-update implementation—the bug this PR is targeting. Please swap the instance in the parent’s children array and retarget each child before calling invalidate.
newRoute.parentRoute = oldRoute.parentRoute
+
+ const parentChildren = newRoute.parentRoute?.children as
+ | Array<AnyRouteWithPrivateProps>
+ | undefined
+ if (parentChildren) {
+ const parentIndex = parentChildren.indexOf(oldRoute as AnyRouteWithPrivateProps)
+ if (parentIndex > -1) {
+ parentChildren[parentIndex] = newRoute
+ }
+ }
+
+ const childRoutes = Array.isArray(newRoute.children)
+ ? (newRoute.children as Array<AnyRouteWithPrivateProps>)
+ : undefined
+ childRoutes?.forEach((child) => {
+ child.parentRoute = newRoute
+ })
const router = window.__TSR_ROUTER__!📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| newRoute.children = oldRoute.children | |
| newRoute.parentRoute = oldRoute.parentRoute | |
| const router = window.__TSR_ROUTER__! | |
| router.routesById[newRoute.id] = newRoute | |
| router.routesByPath[newRoute.fullPath] = newRoute | |
| const oldRouteIndex = router.flatRoutes.indexOf(oldRoute) | |
| if (oldRouteIndex > -1) { | |
| router.flatRoutes[oldRouteIndex] = newRoute | |
| } | |
| router.invalidate({ filter: (m) => m.routeId === oldRoute.id }) | |
| } | |
| newRoute.children = oldRoute.children | |
| newRoute.parentRoute = oldRoute.parentRoute | |
| const parentChildren = newRoute.parentRoute?.children as | |
| | Array<AnyRouteWithPrivateProps> | |
| | undefined | |
| if (parentChildren) { | |
| const parentIndex = parentChildren.indexOf(oldRoute as AnyRouteWithPrivateProps) | |
| if (parentIndex > -1) { | |
| parentChildren[parentIndex] = newRoute | |
| } | |
| } | |
| const childRoutes = Array.isArray(newRoute.children) | |
| ? (newRoute.children as Array<AnyRouteWithPrivateProps>) | |
| : undefined | |
| childRoutes?.forEach((child) => { | |
| child.parentRoute = newRoute | |
| }) | |
| const router = window.__TSR_ROUTER__! | |
| router.routesById[newRoute.id] = newRoute | |
| router.routesByPath[newRoute.fullPath] = newRoute | |
| const oldRouteIndex = router.flatRoutes.indexOf(oldRoute) | |
| if (oldRouteIndex > -1) { | |
| router.flatRoutes[oldRouteIndex] = newRoute | |
| } | |
| router.invalidate({ filter: (m) => m.routeId === oldRoute.id }) | |
| } |
🤖 Prompt for AI Agents
In packages/router-plugin/src/core/route-hmr-statement.ts around lines 19–30,
after copying newRoute.children = oldRoute.children and newRoute.parentRoute =
oldRoute.parentRoute, swap the instance in the parent’s children array (if
parentRoute exists) by finding oldRoute in parent.children and replacing it with
newRoute, then iterate newRoute.children and set each child.parentRoute =
newRoute so children point at the new instance; do these updates before calling
router.invalidate so active matches and tree traversal use the new route
instance.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
fixes #5698
Summary by CodeRabbit
Bug Fixes
Refactor