From e9e2139bf788893566f5a3fe58daf1d24076f018 Mon Sep 17 00:00:00 2001
From: Matthew Phillips <matthew@skypack.dev>
Date: Fri, 23 Aug 2024 08:58:43 -0500
Subject: [PATCH] Remove legacy route prioritization (#11798)

* Remove legacy route prioritization

* oops

* Add a changeset

* Remove bad merge stuff
---
 .changeset/blue-boats-relax.md                |   9 ++
 packages/astro/src/core/config/schema.ts      |   5 -
 .../astro/src/core/routing/manifest/create.ts |  53 +++-----
 packages/astro/src/types/public/config.ts     |  33 +----
 .../astro/src/types/public/integrations.ts    |   9 --
 .../astro/test/units/routing/manifest.test.js | 126 ------------------
 6 files changed, 26 insertions(+), 209 deletions(-)
 create mode 100644 .changeset/blue-boats-relax.md

diff --git a/.changeset/blue-boats-relax.md b/.changeset/blue-boats-relax.md
new file mode 100644
index 0000000000..93d9b51ca2
--- /dev/null
+++ b/.changeset/blue-boats-relax.md
@@ -0,0 +1,9 @@
+---
+'astro': major
+---
+
+Unflag globalRoutePriority
+
+The previously experimental feature `globalRoutePriority` is now the default in Astro 5.
+
+This was a refactoring of route prioritization in Astro, making it so that injected routes, file-based routes, and redirects are all prioritized using the same logic. This feature has been enabled for all Starlight projects since it was added and should not affect most users.
diff --git a/packages/astro/src/core/config/schema.ts b/packages/astro/src/core/config/schema.ts
index 7940e4eb69..abf1be876b 100644
--- a/packages/astro/src/core/config/schema.ts
+++ b/packages/astro/src/core/config/schema.ts
@@ -91,7 +91,6 @@ export const ASTRO_CONFIG_DEFAULTS = {
 		directRenderScript: false,
 		contentCollectionCache: false,
 		clientPrerender: false,
-		globalRoutePriority: false,
 		serverIslands: false,
 		contentIntellisense: false,
 		contentLayer: false,
@@ -527,10 +526,6 @@ export const AstroConfigSchema = z.object({
 				.boolean()
 				.optional()
 				.default(ASTRO_CONFIG_DEFAULTS.experimental.clientPrerender),
-			globalRoutePriority: z
-				.boolean()
-				.optional()
-				.default(ASTRO_CONFIG_DEFAULTS.experimental.globalRoutePriority),
 			serverIslands: z
 				.boolean()
 				.optional()
diff --git a/packages/astro/src/core/routing/manifest/create.ts b/packages/astro/src/core/routing/manifest/create.ts
index 14980f63e5..e45c926af6 100644
--- a/packages/astro/src/core/routing/manifest/create.ts
+++ b/packages/astro/src/core/routing/manifest/create.ts
@@ -9,7 +9,6 @@ import { bold } from 'kleur/colors';
 import { toRoutingStrategy } from '../../../i18n/utils.js';
 import { getPrerenderDefault } from '../../../prerender/utils.js';
 import type { AstroConfig } from '../../../types/public/config.js';
-import type { RoutePriorityOverride } from '../../../types/public/integrations.js';
 import type { RouteData, RoutePart } from '../../../types/public/internal.js';
 import { SUPPORTED_MARKDOWN_FILE_EXTENSIONS } from '../../constants.js';
 import { MissingIndexForInternationalization } from '../../errors/errors-data.js';
@@ -271,18 +270,11 @@ function createFileBasedRoutes(
 	return routes;
 }
 
-type PrioritizedRoutesData = Record<RoutePriorityOverride, RouteData[]>;
-
-function createInjectedRoutes({ settings, cwd }: CreateRouteManifestParams): PrioritizedRoutesData {
+function createInjectedRoutes({ settings, cwd }: CreateRouteManifestParams): RouteData[] {
 	const { config } = settings;
 	const prerender = getPrerenderDefault(config);
 
-	const routes: PrioritizedRoutesData = {
-		normal: [],
-		legacy: [],
-	};
-
-	const priority = computeRoutePriority(config);
+	const routes: RouteData[] = [];
 
 	for (const injectedRoute of settings.injectedRoutes) {
 		const { pattern: name, entrypoint, prerender: prerenderInjected } = injectedRoute;
@@ -317,7 +309,7 @@ function createInjectedRoutes({ settings, cwd }: CreateRouteManifestParams): Pri
 			.map((p) => p.content);
 		const route = joinSegments(segments);
 
-		routes[priority].push({
+		routes.push({
 			type,
 			// For backwards compatibility, an injected route is never considered an index route.
 			isIndex: false,
@@ -343,16 +335,12 @@ function createRedirectRoutes(
 	{ settings }: CreateRouteManifestParams,
 	routeMap: Map<string, RouteData>,
 	logger: Logger,
-): PrioritizedRoutesData {
+): RouteData[] {
 	const { config } = settings;
 	const trailingSlash = config.trailingSlash;
 
-	const routes: PrioritizedRoutesData = {
-		normal: [],
-		legacy: [],
-	};
+	const routes: RouteData[] = [];
 
-	const priority = computeRoutePriority(settings.config);
 	for (const [from, to] of Object.entries(settings.config.redirects)) {
 		const segments = removeLeadingForwardSlash(from)
 			.split(path.posix.sep)
@@ -387,7 +375,7 @@ function createRedirectRoutes(
 			);
 		}
 
-		routes[priority].push({
+		routes.push({
 			type: 'redirect',
 			// For backwards compatibility, a redirect is never considered an index route.
 			isIndex: false,
@@ -505,28 +493,26 @@ export function createRouteManifest(
 	}
 
 	const injectedRoutes = createInjectedRoutes(params);
-	for (const [, routes] of Object.entries(injectedRoutes)) {
-		for (const route of routes) {
-			routeMap.set(route.route, route);
-		}
+	for (const route of injectedRoutes) {
+		routeMap.set(route.route, route);
 	}
 
 	const redirectRoutes = createRedirectRoutes(params, routeMap, logger);
 
 	const routes: RouteData[] = [
-		...injectedRoutes['legacy'].sort(routeComparator),
-		...[...fileBasedRoutes, ...injectedRoutes['normal'], ...redirectRoutes['normal']].sort(
+		...[
+			...fileBasedRoutes,
+			...injectedRoutes,
+			...redirectRoutes
+		].sort(
 			routeComparator,
 		),
-		...redirectRoutes['legacy'].sort(routeComparator),
 	];
 
 	// Report route collisions
-	if (config.experimental.globalRoutePriority) {
-		for (const [index, higherRoute] of routes.entries()) {
-			for (const lowerRoute of routes.slice(index + 1)) {
-				detectRouteCollision(higherRoute, lowerRoute, config, logger);
-			}
+	for (const [index, higherRoute] of routes.entries()) {
+		for (const lowerRoute of routes.slice(index + 1)) {
+			detectRouteCollision(higherRoute, lowerRoute, config, logger);
 		}
 	}
 
@@ -726,13 +712,6 @@ export function createRouteManifest(
 	};
 }
 
-function computeRoutePriority(config: AstroConfig): RoutePriorityOverride {
-	if (config.experimental.globalRoutePriority) {
-		return 'normal';
-	}
-	return 'legacy';
-}
-
 function joinSegments(segments: RoutePart[][]): string {
 	const arr = segments.map((segment) => {
 		return segment.map((rp) => (rp.dynamic ? `[${rp.content}]` : rp.content)).join('');
diff --git a/packages/astro/src/types/public/config.ts b/packages/astro/src/types/public/config.ts
index a2f8e61cc4..78f95869d7 100644
--- a/packages/astro/src/types/public/config.ts
+++ b/packages/astro/src/types/public/config.ts
@@ -11,7 +11,7 @@ import type { AssetsPrefix } from '../../core/app/types.js';
 import type { AstroConfigType } from '../../core/config/schema.js';
 import type { Logger, LoggerLevel } from '../../core/logger/core.js';
 import type { EnvSchema } from '../../env/schema.js';
-import type { AstroIntegration, RoutePriorityOverride } from './integrations.js';
+import type { AstroIntegration } from './integrations.js';
 
 export type Locales = (string | { codes: string[]; path: string })[];
 
@@ -29,7 +29,6 @@ export type RedirectConfig =
 	| {
 			status: ValidRedirectStatus;
 			destination: string;
-			priority?: RoutePriorityOverride;
 	  };
 
 export type ServerConfig = {
@@ -1649,36 +1648,6 @@ export interface AstroUserConfig {
 		 */
 		clientPrerender?: boolean;
 
-		/**
-		 * @docs
-		 * @name experimental.globalRoutePriority
-		 * @type {boolean}
-		 * @default `false`
-		 * @version 4.2.0
-		 * @description
-		 *
-		 * Prioritizes redirects and injected routes equally alongside file-based project routes, following the same [route priority order rules](https://docs.astro.build/en/guides/routing/#route-priority-order) for all routes.
-		 *
-		 * This allows more control over routing in your project by not automatically prioritizing certain types of routes, and standardizes the route priority ordering for all routes.
-		 *
-		 * The following example shows which route will build certain page URLs when file-based routes, injected routes, and redirects are combined as shown below:
-		 * - File-based route: `/blog/post/[pid]`
-		 * - File-based route: `/[page]`
-		 * - Injected route: `/blog/[...slug]`
-		 * - Redirect: `/blog/tags/[tag]` -> `/[tag]`
-		 * - Redirect: `/posts` -> `/blog`
-		 *
-		 * With `experimental.globalRoutingPriority` enabled (instead of Astro 4.0 default route priority order):
-		 *
-		 * - `/blog/tags/astro` is built by the redirect to `/tags/[tag]` (instead of the injected route `/blog/[...slug]`)
-		 * - `/blog/post/0` is built by the file-based route `/blog/post/[pid]` (instead of the injected route `/blog/[...slug]`)
-		 * - `/posts` is built by the redirect to `/blog` (instead of the file-based route `/[page]`)
-		 *
-		 *
-		 * In the event of route collisions, where two routes of equal route priority attempt to build the same URL, Astro will log a warning identifying the conflicting routes.
-		 */
-		globalRoutePriority?: boolean;
-
 		/**
 		 * @docs
 		 * @name experimental.serverIslands
diff --git a/packages/astro/src/types/public/integrations.ts b/packages/astro/src/types/public/integrations.ts
index 129d112158..ea463c9615 100644
--- a/packages/astro/src/types/public/integrations.ts
+++ b/packages/astro/src/types/public/integrations.ts
@@ -136,15 +136,6 @@ export interface AstroInternationalizationFeature {
  */
 export type InjectedScriptStage = 'before-hydration' | 'head-inline' | 'page' | 'page-ssr';
 
-/**
- * IDs for different priorities of injected routes and redirects:
- * - "normal": Merge with discovered file-based project routes, behaving the same as if the route
- *   was defined as a file in the project.
- * - "legacy": Use the old ordering of routes. Inject routes will override any file-based project route,
- *   and redirects will be overridden by any project route on conflict.
- */
-export type RoutePriorityOverride = 'normal' | 'legacy';
-
 export interface InjectedRoute {
 	pattern: string;
 	entrypoint: string;
diff --git a/packages/astro/test/units/routing/manifest.test.js b/packages/astro/test/units/routing/manifest.test.js
index 523d86e6ad..518a2fc8f0 100644
--- a/packages/astro/test/units/routing/manifest.test.js
+++ b/packages/astro/test/units/routing/manifest.test.js
@@ -74,9 +74,6 @@ describe('routing - createRouteManifest', () => {
 			root: fileURLToPath(root),
 			base: '/search',
 			trailingSlash: 'never',
-			experimental: {
-				globalRoutePriority: true,
-			},
 		});
 
 		settings.injectedRoutes = [
@@ -131,9 +128,6 @@ describe('routing - createRouteManifest', () => {
 			root: fileURLToPath(root),
 			base: '/search',
 			trailingSlash: 'never',
-			experimental: {
-				globalRoutePriority: true,
-			},
 		});
 
 		const manifest = createRouteManifest({
@@ -171,9 +165,6 @@ describe('routing - createRouteManifest', () => {
 			root: fileURLToPath(root),
 			base: '/search',
 			trailingSlash: 'never',
-			experimental: {
-				globalRoutePriority: true,
-			},
 		});
 
 		const manifest = createRouteManifest({
@@ -220,9 +211,6 @@ describe('routing - createRouteManifest', () => {
 			root: fileURLToPath(root),
 			base: '/search',
 			trailingSlash: 'never',
-			experimental: {
-				globalRoutePriority: true,
-			},
 		});
 
 		const manifest = createRouteManifest({
@@ -262,58 +250,6 @@ describe('routing - createRouteManifest', () => {
 		]);
 	});
 
-	it('injected routes are sorted in legacy mode above filesystem routes', async () => {
-		const fs = createFs(
-			{
-				'/src/pages/index.astro': `<h1>test</h1>`,
-				'/src/pages/blog/[...slug].astro': `<h1>test</h1>`,
-			},
-			root,
-		);
-		const settings = await createBasicSettings({
-			root: fileURLToPath(root),
-			output: 'server',
-			base: '/search',
-			trailingSlash: 'never',
-		});
-
-		settings.injectedRoutes = [
-			{
-				pattern: '/contributing',
-				entrypoint: '@lib/legacy/static.astro',
-			},
-			{
-				pattern: '/[...slug]',
-				entrypoint: '@lib/legacy/dynamic.astro',
-			},
-		];
-
-		const manifest = createRouteManifest({
-			cwd: fileURLToPath(root),
-			settings,
-			fsMod: fs,
-		});
-
-		assert.deepEqual(getManifestRoutes(manifest), [
-			{
-				route: '/contributing',
-				type: 'page',
-			},
-			{
-				route: '/[...slug]',
-				type: 'page',
-			},
-			{
-				route: '/blog/[...slug]',
-				type: 'page',
-			},
-			{
-				route: '/',
-				type: 'page',
-			},
-		]);
-	});
-
 	it('injected routes are sorted alongside filesystem routes', async () => {
 		const fs = createFs(
 			{
@@ -327,9 +263,6 @@ describe('routing - createRouteManifest', () => {
 			output: 'server',
 			base: '/search',
 			trailingSlash: 'never',
-			experimental: {
-				globalRoutePriority: true,
-			},
 		});
 
 		settings.injectedRoutes = [
@@ -370,53 +303,6 @@ describe('routing - createRouteManifest', () => {
 		]);
 	});
 
-	it('redirects are sorted in legacy mode below the filesystem routes', async () => {
-		const fs = createFs(
-			{
-				'/src/pages/index.astro': `<h1>test</h1>`,
-				'/src/pages/blog/contributing.astro': `<h1>test</h1>`,
-			},
-			root,
-		);
-		const settings = await createBasicSettings({
-			root: fileURLToPath(root),
-			output: 'server',
-			base: '/search',
-			trailingSlash: 'never',
-			redirects: {
-				'/blog/[...slug]': '/',
-				'/blog/about': {
-					status: 302,
-					destination: '/another',
-				},
-			},
-		});
-		const manifest = createRouteManifest({
-			cwd: fileURLToPath(root),
-			settings,
-			fsMod: fs,
-		});
-
-		assert.deepEqual(getManifestRoutes(manifest), [
-			{
-				route: '/blog/contributing',
-				type: 'page',
-			},
-			{
-				route: '/',
-				type: 'page',
-			},
-			{
-				route: '/blog/about',
-				type: 'redirect',
-			},
-			{
-				route: '/blog/[...slug]',
-				type: 'redirect',
-			},
-		]);
-	});
-
 	it('redirects are sorted alongside the filesystem routes', async () => {
 		const fs = createFs(
 			{
@@ -440,9 +326,6 @@ describe('routing - createRouteManifest', () => {
 					destination: '/another',
 				},
 			},
-			experimental: {
-				globalRoutePriority: true,
-			},
 		});
 		const manifest = createRouteManifest({
 			cwd: fileURLToPath(root),
@@ -483,9 +366,6 @@ describe('routing - createRouteManifest', () => {
 			base: '/search',
 			trailingSlash: 'never',
 			integrations: [],
-			experimental: {
-				globalRoutePriority: true,
-			},
 		});
 
 		settings.injectedRoutes = [
@@ -536,9 +416,6 @@ describe('routing - createRouteManifest', () => {
 			base: '/search',
 			trailingSlash: 'never',
 			integrations: [],
-			experimental: {
-				globalRoutePriority: true,
-			},
 		});
 
 		const manifestOptions = {
@@ -585,9 +462,6 @@ describe('routing - createRouteManifest', () => {
 			redirects: {
 				'/posts/a-[b].233': '/blog/a-[b].233',
 			},
-			experimental: {
-				globalRoutePriority: true,
-			},
 		});
 
 		settings.injectedRoutes = [