Skip to content

feat: enhance router to skip catch-all routes for paths with file ext… #1487

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
guitavano wants to merge 5 commits into
base: main
Choose a base branch
from
Open

feat: enhance router to skip catch-all routes for paths with file ext… #1487

Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
509dc73
feat: enhance router to skip catch-all routes for paths with file ext…
guitavano Nov 26, 2025
1312d98
add supportedExtensions
guitavano Nov 26, 2025
92a62bf
change tto router.ts
guitavano Nov 26, 2025
a5ef906
fmt
guitavano Nov 26, 2025
a77479e
use extname from std
guitavano Nov 26, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions website/handlers/router.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,10 @@ const rankRoute = (pattern: string): number =>
return acc + 3;
}, 0);
const urlPatternCache: Record<string, URLPattern> = {};
const hasFileExtension = (pathname: string): boolean => {
const lastSegment = pathname.split("/").pop() || "";
return lastSegment.includes(".") && !lastSegment.startsWith(".");
};
Copy link
Contributor

@coderabbitai coderabbitai bot Nov 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

File-extension skip applies to all pattern routes, not just catch‑alls

The comment says “Skip catch-all routes for paths with file extensions”, but the condition:

if (
  hasFileExtension(url.pathname) &&
  !supportedExtensions?.includes(url.pathname.split(".").pop() || "")
) {
  continue;
}

is evaluated for every non‑isHref route, regardless of whether pathTemplate is catch‑all or a concrete path.

This means:

  • Any pattern-based route (e.g. /sitemap.xml, /feed.rss) that relied on URLPattern matching and does not define supportedExtensions will now be skipped for those extension paths, changing behavior from matching → 404.
  • Only routes that explicitly set supportedExtensions containing the extension will be considered, which is stricter than “just skip catch‑alls”.

If the intent is truly to avoid only wildcard/catch‑all taking over asset-like URLs, consider tightening the condition to check for catch‑all patterns (e.g., routePath.includes("*") or similar), or introducing an explicit opt‑in flag rather than applying this to all routes.

For example:

-    for (
-      const { pathTemplate: routePath, handler, supportedExtensions } of routes
-    ) {
+    for (
+      const { pathTemplate: routePath, handler, supportedExtensions } of routes
+    ) {
       // Skip catch-all routes for paths with file extensions
-      if (
-        hasFileExtension(url.pathname) &&
-        !supportedExtensions?.includes(url.pathname.split(".").pop() || "")
-      ) {
+      const isCatchAll = routePath.includes("*");
+      const lastSegment = url.pathname.split("/").pop() || "";
+      const ext = lastSegment.includes(".")
+        ? lastSegment.split(".").pop() || ""
+        : "";
+      if (
+        isCatchAll &&
+        ext &&
+        !supportedExtensions?.includes(ext)
+      ) {
         continue;
       }

This preserves existing concrete extension routes while still protecting catch‑alls from grabbing asset-like URLs.

Also applies to: 85-94

🤖 Prompt for AI Agents 
In website/handlers/router.ts around lines 42-45 (also apply same change at
85-94): the current file-extension check runs for every pattern route and
wrongly skips concrete extension routes; restrict this guard so it only applies
to catch-all/wildcard pattern routes (e.g., detect patterns with "*" or whatever
identifies a catch-all in this codebase) or add an explicit route opt-in flag,
then only continue when hasFileExtension(url.pathname) && routeIsCatchAll &&
!supportedExtensions?.includes(ext) — leaving concrete extension routes
unaffected.

export const router = (
routes: Route[],
hrefRoutes: Record<string, Resolvable<Handler>> = {},
Expand Down Expand Up @@ -78,6 +82,10 @@ export const router = (
return route(handler, `${url.pathname}${url.search || ""}`);
}
for (const { pathTemplate: routePath, handler } of routes) {
// Skip catch-all routes for paths with file extensions
if (routePath.endsWith("*") && hasFileExtension(url.pathname)) {
continue;
}
const pattern = urlPatternCache[routePath] ??= (() => {
let url;
if (URL.canParse(routePath)) {
Expand Down
Loading