fix(dev): consider pages' route priority when matching request to a p…

…age (#10582)

* fix(dev): consider pages' route priority when matching request to a page

* add test

* add changeset

.changeset/wise-ears-flash.md

@@ -0,0 +1,5 @@
+---
+"astro": patch
+---
+
+Fixes an issue where the dev server got stuck in a loop while routing responses with a 404 status code to the 404 route.

packages/astro/src/core/routing/astro-designed-error-pages.ts

@@ -9,7 +9,7 @@ export function ensure404Route(manifest: ManifestData) {
 			params: [],
 			pattern: /\/404/,
 			prerender: false,
-			segments: [],
+			segments: [[{ content: '404', dynamic: false,spread: false }]],
 			type: 'page',
 			route: '/404',
 			fallbackRoutes: [],

packages/astro/src/core/routing/manifest/create.ts

@@ -21,6 +21,7 @@ import { AstroError } from '../../errors/index.js';
 import { removeLeadingForwardSlash, slash } from '../../path.js';
 import { resolvePages } from '../../util.js';
 import { getRouteGenerator } from './generator.js';
+import { routeComparator } from '../priority.js';
 const require = createRequire(import.meta.url);
 
 interface Item {
@@ -174,111 +175,6 @@ function isSemanticallyEqualSegment(segmentA: RoutePart[], segmentB: RoutePart[]
 	return true;
 }
 
-/**
- * Comparator for sorting routes in resolution order.
- *
- * The routes are sorted in by the following rules in order, following the first rule that
- * applies:
- * - More specific routes are sorted before less specific routes. Here, "specific" means
- *   the number of segments in the route, so a parent route is always sorted after its children.
- *   For example, `/foo/bar` is sorted before `/foo`.
- *   Index routes, originating from a file named `index.astro`, are considered to have one more
- *   segment than the URL they represent.
- * - Static routes are sorted before dynamic routes.
- *   For example, `/foo/bar` is sorted before `/foo/[bar]`.
- * - Dynamic routes with single parameters are sorted before dynamic routes with rest parameters.
- *   For example, `/foo/[bar]` is sorted before `/foo/[...bar]`.
- * - Prerendered routes are sorted before non-prerendered routes.
- * - Endpoints are sorted before pages.
- *   For example, a file `/foo.ts` is sorted before `/bar.astro`.
- * - If both routes are equal regarding all previosu conditions, they are sorted alphabetically.
- *   For example, `/bar` is sorted before `/foo`.
- *   The definition of "alphabetically" is dependent on the default locale of the running system.
- */
-
-function routeComparator(a: RouteData, b: RouteData) {
-	const commonLength = Math.min(a.segments.length, b.segments.length);
-
-	for (let index = 0; index < commonLength; index++) {
-		const aSegment = a.segments[index];
-		const bSegment = b.segments[index];
-
-		const aIsStatic = aSegment.every((part) => !part.dynamic && !part.spread);
-		const bIsStatic = bSegment.every((part) => !part.dynamic && !part.spread);
-
-		if (aIsStatic && bIsStatic) {
-			// Both segments are static, they are sorted alphabetically if they are different
-			const aContent = aSegment.map((part) => part.content).join('');
-			const bContent = bSegment.map((part) => part.content).join('');
-
-			if (aContent !== bContent) {
-				return aContent.localeCompare(bContent);
-			}
-		}
-
-		// Sort static routes before dynamic routes
-		if (aIsStatic !== bIsStatic) {
-			return aIsStatic ? -1 : 1;
-		}
-
-		const aAllDynamic = aSegment.every((part) => part.dynamic);
-		const bAllDynamic = bSegment.every((part) => part.dynamic);
-
-		// Some route might have partial dynamic segments, e.g. game-[title].astro
-		// These routes should have higher priority against route that have **only** dynamic segments, e.g. [title].astro
-		if (aAllDynamic !== bAllDynamic) {
-			return aAllDynamic ? 1 : -1;
-		}
-
-		const aHasSpread = aSegment.some((part) => part.spread);
-		const bHasSpread = bSegment.some((part) => part.spread);
-
-		// Sort dynamic routes with rest parameters after dynamic routes with single parameters
-		// (also after static, but that is already covered by the previous condition)
-		if (aHasSpread !== bHasSpread) {
-			return aHasSpread ? 1 : -1;
-		}
-	}
-
-	const aLength = a.segments.length;
-	const bLength = b.segments.length;
-
-	if (aLength !== bLength) {
-		const aEndsInRest = a.segments.at(-1)?.some((part) => part.spread);
-		const bEndsInRest = b.segments.at(-1)?.some((part) => part.spread);
-
-		if (aEndsInRest !== bEndsInRest && Math.abs(aLength - bLength) === 1) {
-			// If only one of the routes ends in a rest parameter
-			// and the difference in length is exactly 1
-			// and the shorter route is the one that ends in a rest parameter
-			// the shorter route is considered more specific.
-			// I.e. `/foo` is considered more specific than `/foo/[...bar]`
-			if (aLength > bLength && aEndsInRest) {
-				// b: /foo
-				// a: /foo/[...bar]
-				return 1;
-			}
-
-			if (bLength > aLength && bEndsInRest) {
-				// a: /foo
-				// b: /foo/[...bar]
-				return -1;
-			}
-		}
-
-		// Sort routes by length
-		return aLength > bLength ? -1 : 1;
-	}
-
-	// Sort endpoints before pages
-	if ((a.type === 'endpoint') !== (b.type === 'endpoint')) {
-		return a.type === 'endpoint' ? -1 : 1;
-	}
-
-	// Both routes have segments with the same properties
-	return a.route.localeCompare(b.route);
-}
-
 export interface CreateRouteManifestParams {
 	/** Astro Settings object */
 	settings: AstroSettings;

packages/astro/src/core/routing/priority.ts

@@ -0,0 +1,105 @@
+import type { RouteData } from "../../@types/astro.js";
+
+/**
+ * Comparator for sorting routes in resolution order.
+ *
+ * The routes are sorted in by the following rules in order, following the first rule that
+ * applies:
+ * - More specific routes are sorted before less specific routes. Here, "specific" means
+ *   the number of segments in the route, so a parent route is always sorted after its children.
+ *   For example, `/foo/bar` is sorted before `/foo`.
+ *   Index routes, originating from a file named `index.astro`, are considered to have one more
+ *   segment than the URL they represent.
+ * - Static routes are sorted before dynamic routes.
+ *   For example, `/foo/bar` is sorted before `/foo/[bar]`.
+ * - Dynamic routes with single parameters are sorted before dynamic routes with rest parameters.
+ *   For example, `/foo/[bar]` is sorted before `/foo/[...bar]`.
+ * - Prerendered routes are sorted before non-prerendered routes.
+ * - Endpoints are sorted before pages.
+ *   For example, a file `/foo.ts` is sorted before `/bar.astro`.
+ * - If both routes are equal regarding all previosu conditions, they are sorted alphabetically.
+ *   For example, `/bar` is sorted before `/foo`.
+ *   The definition of "alphabetically" is dependent on the default locale of the running system.
+ */
+export function routeComparator(a: RouteData, b: RouteData) {
+	const commonLength = Math.min(a.segments.length, b.segments.length);
+
+	for (let index = 0; index < commonLength; index++) {
+		const aSegment = a.segments[index];
+		const bSegment = b.segments[index];
+
+		const aIsStatic = aSegment.every((part) => !part.dynamic && !part.spread);
+		const bIsStatic = bSegment.every((part) => !part.dynamic && !part.spread);
+
+		if (aIsStatic && bIsStatic) {
+			// Both segments are static, they are sorted alphabetically if they are different
+			const aContent = aSegment.map((part) => part.content).join('');
+			const bContent = bSegment.map((part) => part.content).join('');
+
+			if (aContent !== bContent) {
+				return aContent.localeCompare(bContent);
+			}
+		}
+
+		// Sort static routes before dynamic routes
+		if (aIsStatic !== bIsStatic) {
+			return aIsStatic ? -1 : 1;
+		}
+
+		const aAllDynamic = aSegment.every((part) => part.dynamic);
+		const bAllDynamic = bSegment.every((part) => part.dynamic);
+
+		// Some route might have partial dynamic segments, e.g. game-[title].astro
+		// These routes should have higher priority against route that have **only** dynamic segments, e.g. [title].astro
+		if (aAllDynamic !== bAllDynamic) {
+			return aAllDynamic ? 1 : -1;
+		}
+
+		const aHasSpread = aSegment.some((part) => part.spread);
+		const bHasSpread = bSegment.some((part) => part.spread);
+
+		// Sort dynamic routes with rest parameters after dynamic routes with single parameters
+		// (also after static, but that is already covered by the previous condition)
+		if (aHasSpread !== bHasSpread) {
+			return aHasSpread ? 1 : -1;
+		}
+	}
+
+	const aLength = a.segments.length;
+	const bLength = b.segments.length;
+
+	if (aLength !== bLength) {
+		const aEndsInRest = a.segments.at(-1)?.some((part) => part.spread);
+		const bEndsInRest = b.segments.at(-1)?.some((part) => part.spread);
+
+		if (aEndsInRest !== bEndsInRest && Math.abs(aLength - bLength) === 1) {
+			// If only one of the routes ends in a rest parameter
+			// and the difference in length is exactly 1
+			// and the shorter route is the one that ends in a rest parameter
+			// the shorter route is considered more specific.
+			// I.e. `/foo` is considered more specific than `/foo/[...bar]`
+			if (aLength > bLength && aEndsInRest) {
+				// b: /foo
+				// a: /foo/[...bar]
+				return 1;
+			}
+
+			if (bLength > aLength && bEndsInRest) {
+				// a: /foo
+				// b: /foo/[...bar]
+				return -1;
+			}
+		}
+
+		// Sort routes by length
+		return aLength > bLength ? -1 : 1;
+	}
+
+	// Sort endpoints before pages
+	if ((a.type === 'endpoint') !== (b.type === 'endpoint')) {
+		return a.type === 'endpoint' ? -1 : 1;
+	}
+
+	// Both routes have segments with the same properties
+	return a.route.localeCompare(b.route);
+}

packages/astro/src/prerender/routing.ts

@@ -1,5 +1,6 @@
 import type { AstroSettings, ComponentInstance, RouteData } from '../@types/astro.js';
 import { RedirectComponentInstance, routeIsRedirect } from '../core/redirects/index.js';
+import { routeComparator } from '../core/routing/priority.js';
 import type { DevPipeline } from '../vite-plugin-astro-server/pipeline.js';
 import { getPrerenderStatus } from './metadata.js';
 
@@ -19,7 +20,9 @@ export async function getSortedPreloadedMatches({
 			matches,
 			settings,
 		})
-	).sort((a, b) => prioritizePrerenderedMatchesComparator(a.route, b.route));
+	)
+	.sort((a,b) => routeComparator(a.route, b.route))
+	.sort((a, b) => prioritizePrerenderedMatchesComparator(a.route, b.route))
 }
 
 type PreloadAndSetPrerenderStatusParams = {

packages/astro/src/vite-plugin-astro-server/route.ts

@@ -287,7 +287,6 @@ export async function handleRoute({
 	}
 	if (
 		response.status === 404 &&
-		has404Route(manifestData) &&
 		response.headers.get(REROUTE_DIRECTIVE_HEADER) !== 'no'
 	) {
 		const fourOhFourRoute = await matchRoute('/404', manifestData, pipeline);

packages/astro/test/fixtures/custom-404-loop-case-5/astro.config.mjs

@@ -0,0 +1,3 @@
+export default {
+    output: "server"
+}

packages/astro/test/fixtures/custom-404-loop-case-5/package.json

@@ -0,0 +1,8 @@
+{
+  "name": "@test/custom-404-loop-case-5",
+  "version": "0.0.0",
+  "private": true,
+  "dependencies": {
+    "astro": "workspace:*"
+  }
+}

packages/astro/test/fixtures/custom-404-loop-case-5/src/pages/[x].astro

@@ -0,0 +1,4 @@
+---
+Astro.response.status = 404
+---
+<p>four oh four route</p>