Skip to content

Commit fa50f65

Browse files
committed
Fix route specificity comparison by adding a catch-all detection
This approach is simpler but less robust than a route scoring algorithm - we may have to revisit it later.
1 parent 4b25ef5 commit fa50f65

3 files changed

Lines changed: 199 additions & 20 deletions

File tree

packages/ra-router-tanstack/src/tanStackRouterProvider.spec.tsx

Lines changed: 38 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ import {
2424
NestedResources,
2525
QueryParameters,
2626
PathlessLayoutRoutes,
27+
NestedResourcesPrecedence,
2728
} from './tanStackRouterProvider.stories';
2829
import { tanStackRouterProvider } from './tanStackRouterProvider';
2930

@@ -719,7 +720,9 @@ describe('tanStackRouterProvider', () => {
719720
expect(screen.getByText('Post Details')).toBeInTheDocument();
720721
});
721722
// Check that search params are preserved in location
722-
expect(screen.getByText(/\"search\": \"\?foo=bar\"/)).toBeInTheDocument();
723+
expect(
724+
screen.getByText(/\"search\": \"\?foo=bar\"/)
725+
).toBeInTheDocument();
723726
});
724727

725728
it('should support location object with only search (no pathname)', async () => {
@@ -730,14 +733,20 @@ describe('tanStackRouterProvider', () => {
730733
).toBeInTheDocument();
731734
});
732735

733-
fireEvent.click(screen.getByText('Go to same page with search param'));
736+
fireEvent.click(
737+
screen.getByText('Go to same page with search param')
738+
);
734739

735740
await waitFor(() => {
736741
// Should stay on the same page (Link Tests page)
737-
expect(screen.getByText('Link Component Tests')).toBeInTheDocument();
742+
expect(
743+
screen.getByText('Link Component Tests')
744+
).toBeInTheDocument();
738745
});
739746
// Check that search params are added
740-
expect(screen.getByText(/\"search\": \"\?foo=bar\"/)).toBeInTheDocument();
747+
expect(
748+
screen.getByText(/\"search\": \"\?foo=bar\"/)
749+
).toBeInTheDocument();
741750
});
742751
});
743752

@@ -1442,4 +1451,29 @@ describe('tanStackRouterProvider', () => {
14421451
});
14431452
});
14441453
});
1454+
1455+
describe('Resource Children (Route as children of Resource)', () => {
1456+
it('should navigate to child routes without matching parent edit route', async () => {
1457+
render(<NestedResourcesPrecedence />);
1458+
1459+
// Wait for posts list to load
1460+
await screen.findByText('Post #1');
1461+
1462+
// Click on a post to go to edit page
1463+
fireEvent.click(screen.getByText('Post #1'));
1464+
1465+
// Wait for edit page
1466+
await screen.findByText('Post Details');
1467+
1468+
// Click to view comments (child route)
1469+
fireEvent.click(screen.getByText('View Comments'));
1470+
1471+
// Should navigate to comments page, not stay on edit
1472+
await waitFor(() => {
1473+
expect(
1474+
screen.getByText(/Comments for Post/)
1475+
).toBeInTheDocument();
1476+
});
1477+
});
1478+
});
14451479
});

packages/ra-router-tanstack/src/tanStackRouterProvider.stories.tsx

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1082,6 +1082,69 @@ export const NestedResources = () => (
10821082
</CoreAdmin>
10831083
);
10841084

1085+
const PostEditWithLinkToComments = () => {
1086+
const navigate = useNavigate();
1087+
return (
1088+
<ShowBase
1089+
resource="posts"
1090+
render={({ record }) => (
1091+
<div style={{ padding: 20 }}>
1092+
<h2>Post Details</h2>
1093+
{record && <h3>{record.title}</h3>}
1094+
<button onClick={() => navigate('/posts')}>
1095+
Back to List
1096+
</button>
1097+
<button
1098+
onClick={() => navigate(`/posts/${record.id}/comments`)}
1099+
>
1100+
View Comments
1101+
</button>
1102+
</div>
1103+
)}
1104+
/>
1105+
);
1106+
};
1107+
1108+
const CommentList = () => {
1109+
const { post_id } = useParams();
1110+
const navigate = useNavigate();
1111+
return (
1112+
<ListBase
1113+
resource="comments"
1114+
filter={{ post_id }}
1115+
render={({ data }) => (
1116+
<div style={{ padding: 20 }}>
1117+
<h2>Comments for Post {post_id}</h2>
1118+
<ul>
1119+
{data?.map(record => (
1120+
<li key={record.id}>{record.body}</li>
1121+
))}
1122+
</ul>
1123+
<button onClick={() => navigate(`/posts/${post_id}/show`)}>
1124+
Back to Post
1125+
</button>
1126+
</div>
1127+
)}
1128+
/>
1129+
);
1130+
};
1131+
1132+
export const NestedResourcesPrecedence = () => (
1133+
<CoreAdmin
1134+
routerProvider={tanStackRouterProvider}
1135+
dataProvider={dataProvider}
1136+
layout={LayoutWithLocationDisplay}
1137+
>
1138+
<Resource
1139+
name="posts"
1140+
list={PostList}
1141+
edit={PostEditWithLinkToComments}
1142+
>
1143+
<Route path=":post_id/comments" element={<CommentList />} />
1144+
</Resource>
1145+
</CoreAdmin>
1146+
);
1147+
10851148
/**
10861149
* Tests that query parameters work correctly (for list sorting, filtering, pagination).
10871150
* This tests the navigate({ search: '?...' }) pattern used by useListParams.

packages/ra-router-tanstack/src/tanStackRouterProvider.tsx

Lines changed: 98 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -616,7 +616,7 @@ const Routes = ({ children, location: locationProp }: RouterRoutesProps) => {
616616
if (child.index && (path === '/' || path === '')) {
617617
return true;
618618
}
619-
if (child.path) {
619+
if (child.path !== undefined) {
620620
const match = matchPath({ path: child.path, end: false }, path);
621621
if (match) return true;
622622
}
@@ -628,8 +628,65 @@ const Routes = ({ children, location: locationProp }: RouterRoutesProps) => {
628628
return false;
629629
};
630630

631+
// Check if a route pattern has a catch-all at the end
632+
const hasCatchAll = (path: string): boolean => {
633+
return path.endsWith('/*') || path === '*';
634+
};
635+
636+
// Check if routeB is more specific than routeA when both match the same path
637+
// A route is more specific if it matches more segments with static/param patterns
638+
// before resorting to a catch-all
639+
const isMoreSpecific = (pathA: string, pathB: string): boolean => {
640+
const segmentsA = pathA.split('/').filter(Boolean);
641+
const segmentsB = pathB.split('/').filter(Boolean);
642+
643+
// Count non-catchall segments
644+
const nonCatchallA = segmentsA.filter(s => s !== '*').length;
645+
const nonCatchallB = segmentsB.filter(s => s !== '*').length;
646+
647+
// More non-catchall segments = more specific
648+
if (nonCatchallB > nonCatchallA) return true;
649+
650+
// Same number of non-catchall segments, but B has no catchall while A does
651+
if (
652+
nonCatchallB === nonCatchallA &&
653+
hasCatchAll(pathA) &&
654+
!hasCatchAll(pathB)
655+
) {
656+
return true;
657+
}
658+
659+
// If B has more static segments, it's more specific
660+
const staticA = segmentsA.filter(
661+
s => !s.startsWith(':') && s !== '*'
662+
).length;
663+
const staticB = segmentsB.filter(
664+
s => !s.startsWith(':') && s !== '*'
665+
).length;
666+
if (staticB > staticA) return true;
667+
668+
return false;
669+
};
670+
631671
// Find matching route and calculate the new matched path
632672
const matchResult = useMemo(() => {
673+
// Helper to calculate matched path
674+
const calcMatchedPath = (matchedPortion: string): string => {
675+
if (!parentMatchedPath || parentMatchedPath === '/') {
676+
return matchedPortion;
677+
} else if (matchedPortion === '/') {
678+
return parentMatchedPath;
679+
} else {
680+
return `${parentMatchedPath}${matchedPortion}`;
681+
}
682+
};
683+
684+
let bestMatch: {
685+
route: RouteConfig;
686+
matchedPath: string;
687+
params: Record<string, string | undefined>;
688+
} | null = null;
689+
633690
for (const route of routes) {
634691
if (route.index && (pathname === '/' || pathname === '')) {
635692
// Index route: matched path stays the same
@@ -641,7 +698,6 @@ const Routes = ({ children, location: locationProp }: RouterRoutesProps) => {
641698
// Match if any child route would match the pathname
642699
if (route.path === undefined && route.children) {
643700
if (childRouteMatches(route.children, pathname)) {
644-
// Pathless layout doesn't consume any path
645701
const newMatchedPath = parentMatchedPath || '/';
646702
return { route, matchedPath: newMatchedPath, params: {} };
647703
}
@@ -650,27 +706,46 @@ const Routes = ({ children, location: locationProp }: RouterRoutesProps) => {
650706
if (route.path !== undefined) {
651707
const match = matchPath(route.path, pathname);
652708
if (match) {
653-
// Calculate new matched path by combining parent + matched portion
654709
const matchedPortion = match.pathnameBase || '/';
655-
let newMatchedPath: string;
656-
if (!parentMatchedPath || parentMatchedPath === '/') {
657-
// No parent or root parent - just use matched portion
658-
newMatchedPath = matchedPortion;
659-
} else if (matchedPortion === '/') {
660-
// Matched root - keep parent
661-
newMatchedPath = parentMatchedPath;
662-
} else {
663-
// Combine parent and matched portion
664-
newMatchedPath = `${parentMatchedPath}${matchedPortion}`;
665-
}
666-
return {
710+
const newMatchedPath = calcMatchedPath(matchedPortion);
711+
712+
const currentMatch = {
667713
route,
668714
matchedPath: newMatchedPath,
669715
params: match.params,
670716
};
717+
718+
// If no best match yet, use this one
719+
if (!bestMatch) {
720+
bestMatch = currentMatch;
721+
// If this match doesn't use a catch-all, return immediately
722+
if (!hasCatchAll(route.path)) {
723+
return bestMatch;
724+
}
725+
// Otherwise, keep looking for more specific matches
726+
continue;
727+
}
728+
729+
// Check if this route is more specific than the current best
730+
if (
731+
bestMatch.route.path &&
732+
isMoreSpecific(bestMatch.route.path, route.path)
733+
) {
734+
bestMatch = currentMatch;
735+
// If this match doesn't use a catch-all, return immediately
736+
if (!hasCatchAll(route.path)) {
737+
return bestMatch;
738+
}
739+
}
671740
}
672741
}
673742
}
743+
744+
// If we found a match (possibly a catch-all), return it
745+
if (bestMatch) {
746+
return bestMatch;
747+
}
748+
674749
// Check for catch-all route (path="*")
675750
const catchAll = routes.find(r => r.path === '*');
676751
if (catchAll) {
@@ -682,7 +757,14 @@ const Routes = ({ children, location: locationProp }: RouterRoutesProps) => {
682757
};
683758
}
684759
return null;
685-
}, [routes, pathname, parentMatchedPath, fullPathname]);
760+
}, [
761+
routes,
762+
parentMatchedPath,
763+
pathname,
764+
childRouteMatches,
765+
isMoreSpecific,
766+
fullPathname,
767+
]);
686768

687769
// Now that all hooks have been called, we can safely return early
688770
// if we're outside the basename scope

0 commit comments

Comments
 (0)