Issue: Incorrect route returned for dynamic segment when sibling catch-all segment exists
Closed this issue · 4 comments
Issue: Incorrect route returned for dynamic segment when sibling catch-all segment exists
Description:
The method router.getRouteFromHref(dynamicSegmentHref)
incorrectly returns the route for the catch-all segment instead of the route for the dynamic segment. This issue seems to be related to the sorting function used by the router. It's unclear whether this behavior is a bug or intended.
Steps to Reproduce:
- Define a route configuration where both a dynamic segment and a catch-all segment exist as siblings.
- Use
router.getRouteFromHref(dynamicSegmentHref)
with a URL that matches the dynamic segment. - Observe that the route returned is for the catch-all segment rather than the dynamic segment.
Expected Behavior:
router.getRouteFromHref(dynamicSegmentHref)
should return the route associated with the dynamic segment when a URL matches it, even when a sibling catch-all segment exists.
Actual Behavior:
router.getRouteFromHref(dynamicSegmentHref)
returns the route for the catch-all segment instead of the route for the dynamic segment.
Possible Cause:
The issue may be related to the sorting function used by the router, which might not be correctly prioritizing the dynamic segment over the catch-all segment.
Example Configuration:
roots.config.js
:
const path = require('path')
module.exports = {
originDir: path.resolve(__dirname, 'src/roots'),
localizedDir: path.resolve(__dirname, 'src/app/(routes)'),
locales: ['en', 'cs'],
defaultLocale: 'en',
prefixDefaultLocale: false, // serves "en" locale on / instead of /en
}
src/roots/[dynamic]/page.tsx
:
export default function DynamicPage() { return <></> }
src/roots/[...catchAll]/page.tsx
:
export default function CatchAllPage() { return <></> }
src/roots/page.tsx
:
import { PageProps, Router, schema } from "next-roots";
export default function Home({pageHref}: PageProps) {
const router = new Router(schema);
const locale = router.getLocaleFromHref(pageHref);
const href = router.getHref("/[dynamic]", { locale, dynamic: "page" });
const routeName = router.getRouteFromHref(href)?.name;
return (
<p>Dynamic routeName: "{routeName}"</p>
);
}
Example Usage:
Accessing home page shows: Dynamic routeName: "/[...catchAll]"
Expected Result:
getRouteFromHref
should returns "/[dynamic]"
.
Actual Result:
getRouteFromHref
returns "/[...catchAll]"
.
Suggested Fix:
Review and adjust the sorting function used by the router to ensure that dynamic segments are correctly prioritized over catch-all segments when determining the route. If this behavior is intentional, please could you clarify the design decision ?
I can work on a PR for this.
Thank you!
There is already some sorting mechanism (see
next-roots/src/router/router.ts
Line 76 in 5b659c0
[…
to the end.
IMO we need:
- Non-dynamic routes/segments to come first
- Dynamic but not catch all as second
- Catch all as third
Probably we need to sort routes segment by segment with above rules? But that could harm performance
One thing cane to my mind - we can represent each segment as a number:
- static segment = 1
- dynamic = 2
- catch-all or optimal catch all = 3
For each route we can then compute its weight represented as decimal number like:
/static/[dynamic]
= 0.12
/[dynamic]/[dynamic]
= 0.22
/[dynamic]/static
= 0.21
/[…catchAll]
= 0.3
Then simple sort of decimal numbers can do the trick. We can even calculate this map only once.
What do you think?
Crystal clear! Geat idea.
I can propose a PR for this.
I might need some help to rework the tests for the best. I'll tell you .
🎉 This issue has been resolved in version 3.10.1 🎉
The release is available on:
Your semantic-release bot 📦🚀