Remove legacy route prioritization (#11798)

* Remove legacy route prioritization * oops * Add a changeset * Remove bad merge stuff
2024-12-16 21:46:22 -05:00 · 2024-08-23 08:58:43 -05:00 · 2024-08-23 08:58:43 -05:00 · e9e2139bf7
commit e9e2139bf7
parent ea71b90c9c
6 changed files with 26 additions and 209 deletions
--- a/.changeset/blue-boats-relax.md
+++ 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.
--- 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()
--- 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('');
--- 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
--- 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;
--- 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 = [