fix: prioritize dynamic prerendered routes over dynamic server routes (#7235)

* test: add unit tests

* fix: prioritize prerendered routes

* chore: fix test

* add comment

* test: try avoiding race condition

* chore: changeset

* try avoiding race conditions attempt #2

* try avoiding race conditions (attempt 3)

* final fix hopefuly

* tet: add more tests

* sort conflicting dynamic routes aplhabetically

* test: fix test

.changeset/hungry-peaches-speak.md

@@ -0,0 +1,5 @@
+---
+'astro': patch
+---
+
+Prioritize dynamic prerendered routes over dynamic server routes

packages/astro/src/core/render/dev/index.ts

@@ -1,4 +1,3 @@
-import { fileURLToPath } from 'url';
 import type {
 	AstroMiddlewareInstance,
 	AstroSettings,
@@ -65,7 +64,7 @@ export async function preload({
 
 	try {
 		// Load the module from the Vite SSR Runtime.
-		const mod = (await env.loader.import(fileURLToPath(filePath))) as ComponentInstance;
+		const mod = (await env.loader.import(viteID(filePath))) as ComponentInstance;
 
 		return [renderers, mod];
 	} catch (error) {

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

@@ -173,6 +173,7 @@ function comparator(a: Item, b: Item) {
 		}
 	}
 
+	// endpoints are prioritized over pages
 	if (a.isPage !== b.isPage) {
 		return a.isPage ? 1 : -1;
 	}

packages/astro/src/prerender/routing.ts

@@ -0,0 +1,67 @@
+import type { AstroSettings, RouteData } from '../@types/astro';
+import { preload, type DevelopmentEnvironment } from '../core/render/dev/index.js';
+import { getPrerenderStatus } from './metadata.js';
+
+type GetSortedPreloadedMatchesParams = {
+	env: DevelopmentEnvironment;
+	matches: RouteData[];
+	settings: AstroSettings;
+};
+export async function getSortedPreloadedMatches({
+	env,
+	matches,
+	settings,
+}: GetSortedPreloadedMatchesParams) {
+	return (
+		await preloadAndSetPrerenderStatus({
+			env,
+			matches,
+			settings,
+		})
+	).sort((a, b) => prioritizePrerenderedMatchesComparator(a.route, b.route));
+}
+
+type PreloadAndSetPrerenderStatusParams = {
+	env: DevelopmentEnvironment;
+	matches: RouteData[];
+	settings: AstroSettings;
+};
+async function preloadAndSetPrerenderStatus({
+	env,
+	matches,
+	settings,
+}: PreloadAndSetPrerenderStatusParams) {
+	const preloaded = await Promise.all(
+		matches.map(async (route) => {
+			const filePath = new URL(`./${route.component}`, settings.config.root);
+			const preloadedComponent = await preload({ env, filePath });
+
+			// gets the prerender metadata set by the `astro:scanner` vite plugin
+			const prerenderStatus = getPrerenderStatus({
+				filePath,
+				loader: env.loader,
+			});
+
+			if (prerenderStatus !== undefined) {
+				route.prerender = prerenderStatus;
+			}
+
+			return { preloadedComponent, route, filePath } as const;
+		})
+	);
+	return preloaded;
+}
+
+function prioritizePrerenderedMatchesComparator(a: RouteData, b: RouteData): number {
+	if (areRegexesEqual(a.pattern, b.pattern)) {
+		if (a.prerender !== b.prerender) {
+			return a.prerender ? -1 : 1;
+		}
+		return a.component < b.component ? -1 : 1;
+	}
+	return 0;
+}
+
+function areRegexesEqual(regexp1: RegExp, regexp2: RegExp) {
+	return regexp1.source === regexp2.source && regexp1.global === regexp2.global;
+}

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

@@ -16,10 +16,10 @@ import { preload, renderPage } from '../core/render/dev/index.js';
 import { getParamsAndProps, GetParamsAndPropsError } from '../core/render/index.js';
 import { createRequest } from '../core/request.js';
 import { matchAllRoutes } from '../core/routing/index.js';
-import { getPrerenderStatus } from '../prerender/metadata.js';
 import { isHybridOutput } from '../prerender/utils.js';
 import { log404 } from './common.js';
 import { handle404Response, writeSSRResult, writeWebResponse } from './response.js';
+import { getSortedPreloadedMatches } from '../prerender/routing.js';
 
 type AsyncReturnType<T extends (...args: any) => Promise<any>> = T extends (
 	...args: any
@@ -47,24 +47,12 @@ export async function matchRoute(
 ): Promise<MatchedRoute | undefined> {
 	const { logging, settings, routeCache } = env;
 	const matches = matchAllRoutes(pathname, manifest);
+	const preloadedMatches = await getSortedPreloadedMatches({ env, matches, settings });
 
-	for await (const maybeRoute of matches) {
-		const filePath = new URL(`./${maybeRoute.component}`, settings.config.root);
-		const preloadedComponent = await preload({ env, filePath });
-
-		// gets the prerender metadata set by the `astro:scanner` vite plugin
-		const prerenderStatus = getPrerenderStatus({
-			filePath,
-			loader: env.loader,
-		});
-
-		if (prerenderStatus !== undefined) {
-			maybeRoute.prerender = prerenderStatus;
-		}
-
-		const [, mod] = preloadedComponent;
+	for await (const { preloadedComponent, route: maybeRoute, filePath } of preloadedMatches) {
 		// attempt to get static paths
 		// if this fails, we have a bad URL match!
+		const [, mod] = preloadedComponent;
 		const paramsAndPropsRes = await getParamsAndProps({
 			mod,
 			route: maybeRoute,
@@ -210,7 +198,7 @@ export async function handleRoute(
 			await writeWebResponse(res, result.response);
 		} else {
 			let contentType = 'text/plain';
-			// Dynamic routes don’t include `route.pathname`, so synthesize a path for these (e.g. 'src/pages/[slug].svg')
+			// Dynamic routes don't include `route.pathname`, so synthesize a path for these (e.g. 'src/pages/[slug].svg')
 			const filepath =
 				route.pathname ||
 				route.segments.map((segment) => segment.map((p) => p.content).join('')).join('/');

packages/astro/test/units/routing/route-matching.test.js

@@ -0,0 +1,282 @@
+// @ts-check
+import { createFs, createRequestAndResponse } from '../test-utils.js';
+import { createRouteManifest, matchAllRoutes } from '../../../dist/core/routing/index.js';
+import { fileURLToPath } from 'url';
+import { defaultLogging } from '../../test-utils.js';
+import { createViteLoader } from '../../../dist/core/module-loader/vite.js';
+import { createDevelopmentEnvironment } from '../../../dist/core/render/dev/environment.js';
+import { expect } from 'chai';
+import { createContainer } from '../../../dist/core/dev/container.js';
+import * as cheerio from 'cheerio';
+import testAdapter from '../../test-adapter.js';
+import { getSortedPreloadedMatches } from '../../../dist/prerender/routing.js';
+
+const root = new URL('../../fixtures/alias/', import.meta.url);
+const fileSystem = {
+	'/src/pages/[serverDynamic].astro': `
+		---
+		export const prerender = false;
+		---
+		<p>Server dynamic route! slug:{Astro.params.serverDynamic}</p>
+		`,
+
+	'/src/pages/[xStaticDynamic].astro': `
+		---
+		export function getStaticPaths() {
+			return [
+				{
+					params: {
+						xStaticDynamic: "static-dynamic-route-here",
+					},
+				},
+			];
+		}
+		---
+		<p>Prerendered dynamic route!</p>
+		`,
+	'/src/pages/[aStaticDynamic].astro': `
+			---
+			export function getStaticPaths() {
+				return [
+					{
+						params: {
+							aStaticDynamic: "another-static-dynamic-route-here",
+						},
+					},
+				];
+			}
+			---
+			<p>Another prerendered dynamic route!</p>
+	`,
+	'/src/pages/[...serverRest].astro': `
+	---
+	export const prerender = false;
+	---
+	<p>Server rest route! slug:{Astro.params.serverRest}</p>
+	`,
+	'/src/pages/[...xStaticRest].astro': `
+		---
+		export function getStaticPaths() {
+			return [
+				{
+					params: {
+						xStaticRest: undefined,
+					},
+				},
+			];
+		}
+		---
+		<p>Prerendered rest route!</p>
+`,
+	'/src/pages/[...aStaticRest].astro': `
+		---
+		export function getStaticPaths() {
+			return [
+				{
+					params: {
+						aStaticRest: "another/static-rest-route-here",
+					},
+				},
+			];
+		}
+		---
+		<p>Another prerendered rest route!</p>
+`,
+
+	'/src/pages/nested/[...serverRest].astro': `
+	---
+	export const prerender = false;
+	---
+	<p>Nested server rest route! slug: {Astro.params.serverRest}</p>
+	`,
+	'/src/pages/nested/[...xStaticRest].astro': `
+		---
+		export function getStaticPaths() {
+			return [
+				{
+					params: {
+						xStaticRest: undefined,
+					},
+				},
+			];
+		}
+		---
+		<p>Nested prerendered rest route!</p>
+`,
+	'/src/pages/nested/[...aStaticRest].astro': `
+		---
+		export function getStaticPaths() {
+			return [
+				{
+					params: {
+						aStaticRest: "another-nested-static-dynamic-rest-route-here",
+					},
+				},
+			];
+		}
+		---
+		<p>Another nested prerendered rest route!</p>
+`,
+};
+
+describe('Route matching', () => {
+	let env;
+	let manifest;
+	let container;
+	let settings;
+
+	before(async () => {
+		const fs = createFs(fileSystem, root);
+		container = await createContainer({
+			fs,
+			root,
+			userConfig: {
+				trailingSlash: 'never',
+				output: 'hybrid',
+				experimental: {
+					hybridOutput: true,
+				},
+				adapter: testAdapter(),
+			},
+			disableTelemetry: true,
+		});
+		settings = container.settings;
+
+		const loader = createViteLoader(container.viteServer);
+		env = createDevelopmentEnvironment(container.settings, defaultLogging, loader);
+		manifest = createRouteManifest(
+			{
+				cwd: fileURLToPath(root),
+				settings,
+				fsMod: fs,
+			},
+			defaultLogging
+		);
+	});
+
+	after(async () => {
+		await container.close();
+	});
+
+	describe('Matched routes', () => {
+		it('should be sorted correctly', async () => {
+			const matches = matchAllRoutes('/try-matching-a-route', manifest);
+			const preloadedMatches = await getSortedPreloadedMatches({ env, matches, settings });
+			const sortedRouteNames = preloadedMatches.map((match) => match.route.route);
+
+			expect(sortedRouteNames).to.deep.equal([
+				'/[astaticdynamic]',
+				'/[xstaticdynamic]',
+				'/[serverdynamic]',
+				'/[...astaticrest]',
+				'/[...xstaticrest]',
+				'/[...serverrest]',
+			]);
+		});
+		it('nested should be sorted correctly', async () => {
+			const matches = matchAllRoutes('/nested/try-matching-a-route', manifest);
+			const preloadedMatches = await getSortedPreloadedMatches({ env, matches, settings });
+			const sortedRouteNames = preloadedMatches.map((match) => match.route.route);
+
+			expect(sortedRouteNames).to.deep.equal([
+				'/nested/[...astaticrest]',
+				'/nested/[...xstaticrest]',
+				'/nested/[...serverrest]',
+				'/[...astaticrest]',
+				'/[...xstaticrest]',
+				'/[...serverrest]',
+			]);
+		});
+	});
+
+	describe('Request', () => {
+		it('should correctly match a static dynamic route I', async () => {
+			const { req, res, text } = createRequestAndResponse({
+				method: 'GET',
+				url: '/static-dynamic-route-here',
+			});
+			container.handle(req, res);
+			const html = await text();
+			const $ = cheerio.load(html);
+			expect($('p').text()).to.equal('Prerendered dynamic route!');
+		});
+
+		it('should correctly match a static dynamic route II', async () => {
+			const { req, res, text } = createRequestAndResponse({
+				method: 'GET',
+				url: '/another-static-dynamic-route-here',
+			});
+			container.handle(req, res);
+			const html = await text();
+			const $ = cheerio.load(html);
+			expect($('p').text()).to.equal('Another prerendered dynamic route!');
+		});
+
+		it('should correctly match a server dynamic route', async () => {
+			const { req, res, text } = createRequestAndResponse({
+				method: 'GET',
+				url: '/a-random-slug-was-matched',
+			});
+			container.handle(req, res);
+			const html = await text();
+			const $ = cheerio.load(html);
+			expect($('p').text()).to.equal('Server dynamic route! slug:a-random-slug-was-matched');
+		});
+
+		it('should correctly match a static rest route I', async () => {
+			const { req, res, text } = createRequestAndResponse({
+				method: 'GET',
+				url: '',
+			});
+			container.handle(req, res);
+			const html = await text();
+			const $ = cheerio.load(html);
+			expect($('p').text()).to.equal('Prerendered rest route!');
+		});
+
+		it('should correctly match a static rest route II', async () => {
+			const { req, res, text } = createRequestAndResponse({
+				method: 'GET',
+				url: '/another/static-rest-route-here',
+			});
+			container.handle(req, res);
+			const html = await text();
+			const $ = cheerio.load(html);
+			expect($('p').text()).to.equal('Another prerendered rest route!');
+		});
+
+		it('should correctly match a nested static rest route index', async () => {
+			const { req, res, text } = createRequestAndResponse({
+				method: 'GET',
+				url: '/nested',
+			});
+			container.handle(req, res);
+			const html = await text();
+			const $ = cheerio.load(html);
+			expect($('p').text()).to.equal('Nested prerendered rest route!');
+		});
+
+		it('should correctly match a nested static rest route', async () => {
+			const { req, res, text } = createRequestAndResponse({
+				method: 'GET',
+				url: '/nested/another-nested-static-dynamic-rest-route-here',
+			});
+			container.handle(req, res);
+			const html = await text();
+			const $ = cheerio.load(html);
+			expect($('p').text()).to.equal('Another nested prerendered rest route!');
+		});
+
+		it('should correctly match a nested server rest route', async () => {
+			const { req, res, text } = createRequestAndResponse({
+				method: 'GET',
+				url: '/nested/a-random-slug-was-matched',
+			});
+			container.handle(req, res);
+
+			const html = await text();
+			const $ = cheerio.load(html);
+			expect($('p').text()).to.equal('Nested server rest route! slug: a-random-slug-was-matched');
+		});
+	});
+});