Repository: TanStack/router
Pull request #1584 (2024-05-11 03:20:35)

fix(react-router): implicit navigation for layout routes without a prefixed underscore id 
Layout/pathless routes are denoted by an `id` property.

In file-based routing, an underscore is prefixed to this `id`, but it shouldn't be required.

So the approach for resolving the destination route when navigating without providing the `from` or `to` values needs to be changed to account for this.

nx-cloud[bot]: # ☁️ Nx Cloud Report

CI is running for commit 92d71ef0a7f51d941f3f1b71020cc5ce76f04461.

[📂 Click to track the progress, see the status, the terminal output, and the build insights.](https://cloud.nx.app/cipes/663ee4193f80683fe4d5aee0?utm_source=pull-request&utm_medium=comment)


---

Sent with 💌 from [NxCloud.](https://nx.app?utm_source=pull-request&utm_medium=comment)

Patch:
diff --git a/packages/react-router/src/router.ts b/packages/react-router/src/router.ts
index 6c193bd55..de1bded9e 100644
--- a/packages/react-router/src/router.ts
+++ b/packages/react-router/src/router.ts
@@ -11,7 +11,6 @@ import {
   functionalUpdate,
   last,
   pick,
-  removeLayoutSegments,
   replaceEqualDeep,
 } from './utils'
 import { getRouteMatch } from './RouterProvider'
@@ -494,8 +493,7 @@ export class Router<
       scores: Array<number>
     }> = []
 
-    // eslint-disable-next-line @typescript-eslint/no-unnecessary-type-assertion
-    const routes = Object.values(this.routesById) as Array<AnyRoute>
+    const routes: Array<AnyRoute> = Object.values(this.routesById)
 
     routes.forEach((d, i) => {
       if (d.isRoot || !d.path) {
@@ -937,17 +935,16 @@ export class Router<
         fromMatches.find((e) => e.routeId === d.routeId),
       )
 
-      const fromRouteByFromPath = stayingMatches?.find(
-        (d) => d.pathname === fromPath,
-      )
+      const fromRouteByFromPathRouteId =
+        this.routesById[
+          stayingMatches?.find((d) => d.pathname === fromPath)?.routeId
+        ]
 
       let pathname = dest.to
         ? this.resolvePathWithBase(fromPath, `${dest.to}`)
         : this.resolvePathWithBase(
             fromPath,
-            fromRouteByFromPath
-              ? removeLayoutSegments(fromRouteByFromPath.routeId)
-              : fromPath,
+            fromRouteByFromPathRouteId?.to ?? fromPath,
           )
 
       const prevParams = { ...last(fromMatches)?.params }
diff --git a/packages/react-router/src/utils.ts b/packages/react-router/src/utils.ts
index 3ded3f2ed..6f33f7c18 100644
--- a/packages/react-router/src/utils.ts
+++ b/packages/react-router/src/utils.ts
@@ -339,20 +339,6 @@ export function createControlledPromise<T>(onResolve?: () => void) {
   return controlledPromise
 }
 
-/**
- * Removes all segments from a given path that start with an underscore ('_').
- *
- * @param {string} routePath - The path from which to remove segments. Defaults to '/'.
- * @returns {string} The path with all underscore-prefixed segments removed.
- * @example
- * removeLayoutSegments('/workspace/_auth/foo') // '/workspace/foo'
- */
-export function removeLayoutSegments(routePath: string): string {
-  const segments = routePath.split('/')
-  const newSegments = segments.filter((segment) => !segment.startsWith('_'))
-  return newSegments.join('/')
-}
-
 /**
  * Taken from https://www.developerway.com/posts/implementing-advanced-use-previous-hook#part3
  */


Test patch:
diff --git a/packages/react-router/tests/redirects.test.ts b/packages/react-router/tests/redirects.test.ts
index 7c0b35cce..75643cc9e 100644
--- a/packages/react-router/tests/redirects.test.ts
+++ b/packages/react-router/tests/redirects.test.ts
@@ -1,12 +1,11 @@
 import { describe, expect, it } from 'vitest'
 
 import {
-  type RouterHistory,
   createMemoryHistory,
   createRootRoute,
   createRoute,
   createRouter,
-  redirect,
+  type RouterHistory,
 } from '../src'
 
 function createTestRouter(initialHistory?: RouterHistory) {
@@ -40,16 +39,29 @@ function createTestRouter(initialHistory?: RouterHistory) {
     path: '/$framework',
   })
 
-  const userRoute = createRoute({
+  const uRoute = createRoute({
     getParentRoute: () => rootRoute,
     path: '/u',
   })
-  const userLayoutRoute = createRoute({
+  const uLayoutRoute = createRoute({
     id: '_layout',
-    getParentRoute: () => userRoute,
+    getParentRoute: () => uRoute,
   })
-  const usernameRoute = createRoute({
-    getParentRoute: () => userLayoutRoute,
+  const uUsernameRoute = createRoute({
+    getParentRoute: () => uLayoutRoute,
+    path: '$username',
+  })
+
+  const gRoute = createRoute({
+    getParentRoute: () => rootRoute,
+    path: '/g',
+  })
+  const gLayoutRoute = createRoute({
+    id: 'layout',
+    getParentRoute: () => gRoute,
+  })
+  const gUsernameRoute = createRoute({
+    getParentRoute: () => gLayoutRoute,
     path: '$username',
   })
 
@@ -58,15 +70,15 @@ function createTestRouter(initialHistory?: RouterHistory) {
       projectVersionRoute.addChildren([projectFrameRoute]),
     ]),
   ])
-  const userTree = userRoute.addChildren([
-    userLayoutRoute.addChildren([usernameRoute]),
-  ])
+  const uTree = uRoute.addChildren([uLayoutRoute.addChildren([uUsernameRoute])])
+  const gTree = gRoute.addChildren([gLayoutRoute.addChildren([gUsernameRoute])])
 
   const routeTree = rootRoute.addChildren([
     indexRoute,
     postsRoute.addChildren([postIdRoute]),
     projectTree,
-    userTree,
+    uTree,
+    gTree,
   ])
   const router = createRouter({ routeTree, history })
 
@@ -407,4 +419,39 @@ describe('router.navigate navigation using layout routes resolves correctly', as
 
     expect(router.state.location.pathname).toBe('/u/tkdodo')
   })
+
+  it('should resolve "/g/tanner" in "/g/layout/$username" to "/g/tkdodo"', async () => {
+    const { router } = createTestRouter(
+      createMemoryHistory({ initialEntries: ['/g/tanner'] }),
+    )
+
+    await router.load()
+
+    expect(router.state.location.pathname).toBe('/g/tanner')
+
+    await router.navigate({
+      to: '/g/$username',
+      params: { username: 'tkdodo' },
+    })
+    await router.invalidate()
+
+    expect(router.state.location.pathname).toBe('/g/tkdodo')
+  })
+
+  it('should resolve "/g/tanner" in "/g/layout/$username" to "/g/tkdodo" w/o "to" path being provided', async () => {
+    const { router } = createTestRouter(
+      createMemoryHistory({ initialEntries: ['/g/tanner'] }),
+    )
+
+    await router.load()
+
+    expect(router.state.location.pathname).toBe('/g/tanner')
+
+    await router.navigate({
+      params: { username: 'tkdodo' },
+    })
+    await router.invalidate()
+
+    expect(router.state.location.pathname).toBe('/g/tkdodo')
+  })
 })
diff --git a/packages/react-router/tests/utils.test.ts b/packages/react-router/tests/utils.test.ts
index e882233ab..ea0f4b8d0 100644
--- a/packages/react-router/tests/utils.test.ts
+++ b/packages/react-router/tests/utils.test.ts
@@ -2,7 +2,6 @@ import { describe, expect, it } from 'vitest'
 import {
   exactPathTest,
   isPlainArray,
-  removeLayoutSegments,
   removeTrailingSlash,
   replaceEqualDeep,
 } from '../src/utils'
@@ -375,53 +374,3 @@ describe('exactPathTest', () => {
     expect(result).toBe(true)
   })
 })
-
-describe('removeLayoutSegments', () => {
-  it('should remove the "_layout" segment from "/_layout/" and return "/"', () => {
-    const path = '/_layout/'
-    const result = removeLayoutSegments(path)
-    expect(result).toBe('/')
-  })
-
-  it('should remove the "_layout" segment from "/_layout/blog" and return "/blog"', () => {
-    const path = '/_layout/blog'
-    const result = removeLayoutSegments(path)
-    expect(result).toBe('/blog')
-  })
-
-  it('should remove the "_layout1" and "_layout2" segments from "/_layout1/" and return "/"', () => {
-    const path = '/_layout1/_layout2/'
-    const result = removeLayoutSegments(path)
-    expect(result).toBe('/')
-  })
-
-  it('should remove the "_layout1" and "_layout2" segments from "/_layout1/blog" and return "/blog"', () => {
-    const path = '/_layout1/_layout2/blog'
-    const result = removeLayoutSegments(path)
-    expect(result).toBe('/blog')
-  })
-
-  it('should remove the "_layout" segment from "/posts/_layout/" and return "/posts/"', () => {
-    const path = '/posts/_layout/1'
-    const result = removeLayoutSegments(path)
-    expect(result).toBe('/posts/1')
-  })
-
-  it('should remove the "_layout" segment from "/posts/_layout/1" and return "/posts/1"', () => {
-    const path = '/posts/_layout/1'
-    const result = removeLayoutSegments(path)
-    expect(result).toBe('/posts/1')
-  })
-
-  it('should remove the "_layout" segment from "/posts/_layout/" and return "/posts"', () => {
-    const path = '/posts/_layout/'
-    const result = removeLayoutSegments(path)
-    expect(result).toBe('/posts/')
-  })
-
-  it('should remove the "_layout" segment from "/posts/_layout/blog" and return "/posts/blog"', () => {
-    const path = '/posts/_layout/blog'
-    const result = removeLayoutSegments(path)
-    expect(result).toBe('/posts/blog')
-  })
-})