From 5b29550996a7f5459a0d611feea6e51d44e1d8ed Mon Sep 17 00:00:00 2001
From: Luiz Ferraz <luiz@lferraz.com>
Date: Tue, 23 Jan 2024 21:23:10 -0300
Subject: [PATCH] Fix regression on dynamic sibling trees and index inside rest
 parameter folders (#9786)

* fix: Fix regression on dynamic sibling trees and index inside rest parameter folders

* Add extra test scenarios

* Make `/[foo].astro` also win over `/[foo]/[...rest].astro`

* Make `/[foo].astro` also win over `/[foo]/[...rest].astro`

* Update tests

* Remove commented out code

* Update .changeset/six-shrimps-glow.md

* Fix sorting cycle

---------

Co-authored-by: Nate Moore <natemoo-re@users.noreply.github.com>
---
 .changeset/six-shrimps-glow.md                |  39 +++++++
 .../astro/src/core/routing/manifest/create.ts | 109 +++++++++++++-----
 packages/astro/src/prerender/utils.ts         |   2 +-
 .../astro/test/units/routing/manifest.test.js |  97 ++++++++++++++--
 4 files changed, 204 insertions(+), 43 deletions(-)
 create mode 100644 .changeset/six-shrimps-glow.md

diff --git a/.changeset/six-shrimps-glow.md b/.changeset/six-shrimps-glow.md
new file mode 100644
index 0000000000..2d925bb8c1
--- /dev/null
+++ b/.changeset/six-shrimps-glow.md
@@ -0,0 +1,39 @@
+---
+"astro": patch
+---
+
+Fixes a regression in routing priority for index pages in rest parameter folders and dynamic sibling trees.
+
+Considering the following tree:
+```
+src/pages/
+├── index.astro
+├── static.astro
+├── [dynamic_file].astro
+├── [...rest_file].astro
+├── blog/
+│   └── index.astro
+├── [dynamic_folder]/
+│   ├── index.astro
+│   ├── static.astro
+│   └── [...rest].astro
+└── [...rest_folder]/
+    ├── index.astro
+    └── static.astro
+```
+
+The routes are sorted in this order:
+```
+/src/pages/index.astro
+/src/pages/blog/index.astro
+/src/pages/static.astro
+/src/pages/[dynamic_folder]/index.astro
+/src/pages/[dynamic_file].astro
+/src/pages/[dynamic_folder]/static.astro
+/src/pages/[dynamic_folder]/[...rest].astro
+/src/pages/[...rest_folder]/static.astro
+/src/pages/[...rest_folder]/index.astro
+/src/pages/[...rest_file]/index.astro
+```
+
+This allows for index files to be used as overrides to rest parameter routes on SSR when the rest parameter matching `undefined` is not desired.
diff --git a/packages/astro/src/core/routing/manifest/create.ts b/packages/astro/src/core/routing/manifest/create.ts
index 6ab297a5ee..1b4909a2e3 100644
--- a/packages/astro/src/core/routing/manifest/create.ts
+++ b/packages/astro/src/core/routing/manifest/create.ts
@@ -194,47 +194,94 @@ function isSemanticallyEqualSegment(segmentA: RoutePart[], segmentB: RoutePart[]
  *   The definition of "alphabetically" is dependent on the default locale of the running system.
  */
 function routeComparator(a: ManifestRouteData, b: ManifestRouteData) {
+	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 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;
+		}
+	}
+
+	// Special case to have `/[foo].astro` be equivalent to `/[foo]/index.astro`
+	// when compared against `/[foo]/[...rest].astro`.
+	if (Math.abs(a.segments.length - b.segments.length) === 1) {
+		const aEndsInRest = a.segments.at(-1)?.some((part) => part.spread);
+		const bEndsInRest = b.segments.at(-1)?.some((part) => part.spread);
+
+		// Routes with rest parameters are less specific than their parent route.
+		// For example, `/foo/[...bar]` is sorted after `/foo`.
+
+		if (a.segments.length > b.segments.length && !bEndsInRest) {
+			return 1;
+		}
+
+		if (b.segments.length > a.segments.length && !aEndsInRest) {
+			return -1;
+		}
+	}
+
+	if (a.isIndex !== b.isIndex) {
+		// Index pages are lower priority than other static segments in the same prefix.
+		// They match the path up to their parent, but are more specific than the parent.
+		// For example:
+		//  - `/foo/index.astro` is sorted before `/foo`
+		//  - `/foo/index.astro` is sorted before `/foo/[bar].astro`
+		//  - `/[...foo]/index.astro` is sorted after `/[...foo]/bar.astro`
+
+		if (a.isIndex) {
+			const followingBSegment = b.segments.at(a.segments.length);
+			const followingBSegmentIsStatic = followingBSegment?.every((part) => !part.dynamic && !part.spread);
+
+			return followingBSegmentIsStatic ? 1 : -1;
+		}
+
+		const followingASegment = a.segments.at(b.segments.length);
+		const followingASegmentIsStatic = followingASegment?.every((part) => !part.dynamic && !part.spread);
+
+		return followingASegmentIsStatic ? -1 : 1;
+	}
+
 	// For sorting purposes, an index route is considered to have one more segment than the URL it represents.
 	const aLength = a.isIndex ? a.segments.length + 1 : a.segments.length;
 	const bLength = b.isIndex ? b.segments.length + 1 : b.segments.length;
-
-	// Sort more specific routes before less specific routes
-	if (aLength !== bLength) {
+	
+	if (aLength !== bLength){
+		// Routes are equal up to the smaller of the two lengths, so the longer route is more specific
 		return aLength > bLength ? -1 : 1;
 	}
-
-	const aIsStatic = a.segments.every((segment) =>
-		segment.every((part) => !part.dynamic && !part.spread)
-	);
-	const bIsStatic = b.segments.every((segment) =>
-		segment.every((part) => !part.dynamic && !part.spread)
-	);
-
-	// Sort static routes before dynamic routes
-	if (aIsStatic !== bIsStatic) {
-		return aIsStatic ? -1 : 1;
-	}
-
-	const aHasSpread = a.segments.some((segment) => segment.some((part) => part.spread));
-	const bHasSpread = b.segments.some((segment) => segment.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;
-	}
-
-	// Sort prerendered routes before non-prerendered routes
-	if (a.prerender !== b.prerender) {
-		return a.prerender ? -1 : 1;
-	}
-
+	
 	// Sort endpoints before pages
 	if ((a.type === 'endpoint') !== (b.type === 'endpoint')) {
 		return a.type === 'endpoint' ? -1 : 1;
 	}
 
-	// Sort alphabetically
+	// Both routes have segments with the same properties
 	return a.route.localeCompare(b.route);
 }
 
diff --git a/packages/astro/src/prerender/utils.ts b/packages/astro/src/prerender/utils.ts
index b444352135..5cc6273430 100644
--- a/packages/astro/src/prerender/utils.ts
+++ b/packages/astro/src/prerender/utils.ts
@@ -6,7 +6,7 @@ export function isServerLikeOutput(config: AstroConfig) {
 }
 
 export function getPrerenderDefault(config: AstroConfig) {
-	return config.output === 'hybrid';
+	return config.output !== 'server';
 }
 
 /**
diff --git a/packages/astro/test/units/routing/manifest.test.js b/packages/astro/test/units/routing/manifest.test.js
index 7bfdf68391..3ad1ab302e 100644
--- a/packages/astro/test/units/routing/manifest.test.js
+++ b/packages/astro/test/units/routing/manifest.test.js
@@ -52,8 +52,8 @@ describe('routing - createRouteManifest', () => {
 	it('endpoint routes are sorted before page routes', async () => {
 		const fs = createFs(
 			{
-				'/src/pages/contact-me.astro': `<h1>test</h1>`,
-				'/src/pages/sendContact.ts': `<h1>test</h1>`,
+				'/src/pages/[contact].astro': `<h1>test</h1>`,
+				'/src/pages/[contact].ts': `<h1>test</h1>`,
 			},
 			root
 		);
@@ -84,20 +84,20 @@ describe('routing - createRouteManifest', () => {
 		});
 
 		expect(getManifestRoutes(manifest)).to.deep.equal([
-			{
-				route: '/api',
-				type: 'endpoint',
-			},
-			{
-				route: '/sendcontact',
-				type: 'endpoint',
-			},
 			{
 				route: '/about',
 				type: 'page',
 			},
 			{
-				route: '/contact-me',
+				route: '/api',
+				type: 'endpoint',
+			},
+			{
+				route: '/[contact]',
+				type: 'endpoint',
+			},
+			{
+				route: '/[contact]',
 				type: 'page',
 			},
 		]);
@@ -148,6 +148,81 @@ describe('routing - createRouteManifest', () => {
 		]);
 	});
 
+	it('route sorting respects the file tree', async () => {
+		const fs = createFs(
+			{
+				'/src/pages/[dynamic_folder]/static.astro': `<h1>test</h1>`,
+				'/src/pages/[dynamic_folder]/index.astro': `<h1>test</h1>`,
+				'/src/pages/[dynamic_folder]/[...rest].astro': `<h1>test</h1>`,
+				'/src/pages/[...rest]/static.astro': `<h1>test</h1>`,
+				'/src/pages/[...rest]/index.astro': `<h1>test</h1>`,
+				'/src/pages/blog/index.astro': `<h1>test</h1>`,
+				'/src/pages/[dynamic_file].astro': `<h1>test</h1>`,
+				'/src/pages/[...other].astro': `<h1>test</h1>`,
+				'/src/pages/static.astro': `<h1>test</h1>`,
+				'/src/pages/index.astro': `<h1>test</h1>`,
+			},
+			root
+		);
+		const settings = await createBasicSettings({
+			root: fileURLToPath(root),
+			base: '/search',
+			trailingSlash: 'never',
+			experimental: {
+				globalRoutePriority: true,
+			},
+		});
+
+		const manifest = createRouteManifest({
+			cwd: fileURLToPath(root),
+			settings,
+			fsMod: fs,
+		});
+
+		expect(getManifestRoutes(manifest)).to.deep.equal([
+			{
+				"route": "/",
+				"type": "page",
+			},
+			{
+				"route": "/blog",
+				"type": "page",
+			},
+			{
+				"route": "/static",
+				"type": "page",
+			},
+			{
+				"route": "/[dynamic_folder]",
+				"type": "page",
+			},
+			{
+				"route": "/[dynamic_file]",
+				"type": "page",
+			},
+			{
+				"route": "/[dynamic_folder]/static",
+				"type": "page",
+			},
+			{
+				"route": "/[dynamic_folder]/[...rest]",
+				"type": "page",
+			},
+			{
+				"route": "/[...rest]/static",
+				"type": "page",
+			},
+			{
+				"route": "/[...rest]",
+				"type": "page",
+			},
+			{
+				"route": "/[...other]",
+				"type": "page",
+			},
+		]);
+	});
+
 	it('injected routes are sorted in legacy mode above filesystem routes', async () => {
 		const fs = createFs(
 			{