Trim parent route id prefix when creating lazy routes #2401
Conversation
|
can you please add a test for this? |
☁️ Nx Cloud ReportCI is running/has finished running commands for commit d047bb9. As they complete they will appear below. Click to see the status, the terminal output, and the build insights. 📂 See all runs for this CI Pipeline Execution
✅ Successfully ran 1 targetSent with 💌 from NxCloud. |
No problem, I'll do it later today. Actually I found this fix won't work for lazy pathless routes, since they don't have path, will always use |
stevenlyd
changed the title
|
@schiller-manuel |
stevenlyd
changed the title
| @@ -276,5 +280,8 @@ export function createLazyFileRoute< | |||
| TRoute extends FileRoutesByPath[TFilePath]['preLoaderRoute'], | |||
| >(id: TFilePath) { | |||
| return (opts: LazyRouteOptions) => | |||
| new LazyRoute<TRoute>({ id: removeGroups(id), ...opts }) | |||
| new LazyRoute<TRoute>({ | |||
| id: getLastPathSegment(removeGroups(id)) as any, | |||
There was a problem hiding this comment.
i am wondering about why the last path segment should be used as id.
this might work, but aren't there cases where this is not unique?
e.g. consider the following routes:
/foo/bar
/hello/bar
both would end up with the same id, right?
There was a problem hiding this comment.
Yes, you're right that both /foo/bar and /hello/bar have the same last path segment bar. However, this only affects the options.id of the lazy route object. From what I understand, the options.id in the lazy route object is mainly used during the preloading of a route. When a route is preloaded, these options are assigned back to the primary route object like this:
// ./packages/react-router/src/router.ts
route._lazyPromise =
route._lazyPromise ||
(route.lazyFn
? route.lazyFn().then((lazyRoute) => {
Object.assign(route.options, lazyRoute.options)
})
: Promise.resolve())I believe that the options.id should represent the route's own ID, not the full ID that includes prefixes from parent routes. This seems to be consistent with how IDs and paths got updated for primary route objects in routeTree.gen.ts. For example:
const ProtectedRouteRoute = ProtectedRouteImport.update({
id: '/_protected',
getParentRoute: () => rootRoute,
} as any)
const ProtectedLayoutRouteRoute = ProtectedLayoutRouteImport.update({
id: '/_layout', // Full id should be /_protected/_layout
getParentRoute: () => ProtectedRouteRoute,
} as any)
const ProtectedLayoutParentRouteRoute = ProtectedLayoutParentRouteImport.update(
{
path: '/parent',
getParentRoute: () => ProtectedLayoutRouteRoute,
} as any,
)
const ProtectedLayoutParentTestRouteRoute =
ProtectedLayoutParentTestRouteImport.update({
path: '/test', // Full path should be /parent/test
getParentRoute: () => ProtectedLayoutParentRouteRoute,
} as any).lazy(() =>
import('./routes/_protected/_layout/parent/test/route.lazy').then(
(d) => d.Route,
),
)I guess the reason why we assign the full ID at the beginning but update it to a trimmed version later is that the code generator needs to consume that full path or full ID information when auto-generating the route tree.
If I am not mistaken, it's precisely because the id and path in primary route objects are updated to their trimmed versions in routeTree.gen.ts that TSR can function properly. Otherwise if they are in full version, the route.id we generate initially would be incorrect, containing duplicate parent prefixes. However, we currently don't trim the options.id for the lazy route object, which can cause issues when it gets assigned back to the primary route object.
Of course, these incorrect options.id values only take effect when we run router.buildRouteTree again. Since few people rerun router.buildRouteTree after the router is instantiated, this issue hasn't been discovered until now.
From my understanding, route.options.id represents the route's own ID, whereas route.id is the full ID that exists only in the primary route object and is generated when route.init is called. This process combines the customId with the parent's full ID as a prefix. Additionally, the customId is derived from either the path (not fullPath) or options.id, depending on whether the route is pathless.
As a result, the route's ID ultimately become /foo/bar and /hello/bar, maintaining uniqueness even if the last segments are the same.
There was a problem hiding this comment.
can you add a test for this as well please?
There was a problem hiding this comment.
Sure, you mean a test that checks if the lazy route object getting the trimmed version of id?
There was a problem hiding this comment.
and that IDs are still unique then
There was a problem hiding this comment.
and that IDs are still unique then
I've added tests for route ID uniqueness, and also added routes ending with same last segments to that test router.
There was a problem hiding this comment.
and that IDs are still unique then
And also added tests that check if the lazy route's options.id is trimmed.
#2400