fix(astro): Fixes prerender conflicts of lower-priority routes

.changeset/tall-tools-share.md

@@ -0,0 +1,5 @@
+---
+'astro': patch
+---
+
+Fixes bug where lower priority prerendered routes were generated on top of higher priority SSR routes.

packages/astro/src/core/build/generate.ts

@@ -37,7 +37,7 @@
 import { RenderContext } from '../render-context.js';
 import { callGetStaticPaths } from '../render/route-cache.js';
 import { createRequest } from '../request.js';
-import { matchRoute } from '../routing/match.js';
+import { matchAllRoutesOrFallback, matchRoute } from '../routing/match.js';
 import { stringifyParams } from '../routing/params.js';
 import { getOutputFilename, isServerLikeOutput } from '../util.js';
 import { getOutDirWithinCwd, getOutFile, getOutFolder } from './common.js';
@@ -97,7 +97,7 @@
 
 	const verb = ssr ? 'prerendering' : 'generating';
 	logger.info('SKIP_FORMAT', `\n${bgGreen(black(` ${verb} static routes `))}`);
-	const builtPaths = new Set<string>();
+	const builtPaths = new Map<string, RouteData>();
 	const pagesToGenerate = pipeline.retrieveRoutesToGenerate();
 	if (ssr) {
 		for (const [pageData, filePath] of pagesToGenerate) {
@@ -171,7 +171,7 @@
 async function generatePage(
 	pageData: PageBuildData,
 	ssrEntry: SinglePageBuiltModule,
-	builtPaths: Set<string>,
+	builtPaths: Map<string, RouteData>,
 	pipeline: BuildPipeline,
 ) {
 	// prepare information we need
@@ -269,17 +269,26 @@
 	}
 }
 
+function isRoute(target: RouteData, reference: RouteData): boolean {
+	return target === reference || reference.fallbackRoutes.includes(target);
+}
+
 async function getPathsForRoute(
 	route: RouteData,
 	mod: ComponentInstance,
 	pipeline: BuildPipeline,
-	builtPaths: Set<string>,
+	builtPaths: Map<string, RouteData>,
 ): Promise<Array<string>> {
 	const { logger, options, routeCache, serverLike } = pipeline;
 	let paths: Array<string> = [];
 	if (route.pathname) {
-		paths.push(route.pathname);
-		builtPaths.add(removeTrailingForwardSlash(route.pathname));
+		const path = route.generate({});
+		const matchedRoute = matchRoute(path, options.manifest) ?? route;
+		if (isRoute(route, matchedRoute)) {
+			paths.push(path);
+		} else {
+			logger.warn('build', `Skipping conflicting route for "${path}" (${route.component})`);
+		}
 	} else {
 		const staticPaths = await callGetStaticPaths({
 			mod,
@@ -310,26 +319,49 @@
 				}
 			})
 			.filter((staticPath) => {
-				// The path hasn't been built yet, include it
-				if (!builtPaths.has(removeTrailingForwardSlash(staticPath))) {
-					return true;
-				}
-
-				// The path was already built once. Check the manifest to see if
-				// this route takes priority for the final URL.
+				// Check the manifest to see if this route is the highest priority matching
+				// the final URL. If it isn't, skip it.
 				// NOTE: The same URL may match multiple routes in the manifest.
 				// Routing priority needs to be verified here for any duplicate
 				// paths to ensure routing priority rules are enforced in the final build.
-				const matchedRoute = matchRoute(staticPath, options.manifest);
-				return matchedRoute === route;
+				const matchedRoutes = matchAllRoutesOrFallback(staticPath, options.manifest)
+
+				// Route that previously rendered this path
+				const renderingRoute = builtPaths.get(staticPath);
+
+				for (const matchedRoute of matchedRoutes) {
+					// This is the highest priority matching route
+					if (isRoute(route, matchedRoute)) {
+						return true;
+					}
+
+					// There is an on-demand route matching this path with a higher priority
+					// skip rendering this path
+					if (!matchedRoute.prerender) {
+						logger.warn('build', `Skipped building path "${staticPath}" due to on-demand route "${matchedRoute.route}"`);
+						return false;
+					}
+
+					// Path was already pre-rendered by a higher priority route
+					if (matchedRoute === renderingRoute) {
+						logger.warn('build', `Skipped building path "${staticPath}" due to higher priority pre-rendered route "${matchedRoute.route}"`);
+						return false;
+					}
+				}
+
+				// Route not matched, skip it
+				logger.error('build', `Route (${route.component}) generated path (${staticPath}) that doesn't match its own pattern (${route.route})`);
+				return false;
 			});
+	}
 
-		// Add each path to the builtPaths set, to avoid building it again later.
-		for (const staticPath of paths) {
-			builtPaths.add(removeTrailingForwardSlash(staticPath));
-		}
+	// Add each path to the builtPaths set, to avoid building it again later.
+	for (const staticPath of paths) {
+		builtPaths.set(staticPath, route);
 	}
 
+	console.log('building paths', paths);
+
 	return paths;
 }
 
@@ -355,10 +387,10 @@
 		...AstroErrorData.InvalidDynamicRoute,
 		message: invalidParam
 			? AstroErrorData.InvalidDynamicRoute.message(
-					route.route,
-					JSON.stringify(invalidParam),
-					JSON.stringify(received),
-				)
+				route.route,
+				JSON.stringify(invalidParam),
+				JSON.stringify(received),
+			)
 			: `Generated path for ${route.route} is invalid.`,
 		hint,
 	});

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

@@ -11,6 +11,17 @@ export function matchRoute(pathname: string, manifest: ManifestData): RouteData
 	});
 }
 
+/** Finds all matching routes from pathname */
+export function matchAllRoutesOrFallback(pathname: string, manifest: ManifestData): RouteData[] {
+	const decodedPathname = decodeURI(pathname);
+	return manifest.routes.filter((route) => {
+		return (
+			route.pattern.test(decodedPathname) ||
+			route.fallbackRoutes.some((fallbackRoute) => fallbackRoute.pattern.test(decodedPathname))
+		);
+	});
+}
+
 /** Finds all matching routes from pathname */
 export function matchAllRoutes(pathname: string, manifest: ManifestData): RouteData[] {
 	return manifest.routes.filter((route) => route.pattern.test(decodeURI(pathname)));

packages/astro/test/fixtures/ssr-prerender/src/pages/[...slug].astro

@@ -0,0 +1,11 @@
+---
+export const prerender = true;
+
+export const getStaticPaths = () => ([
+	{ params: { slug: 'static' } },
+	{ params: { slug: 'not-prerendered' } },
+]);
+---
+<div>
+  <p>Prerendered dynamic route.</p>
+</div>

packages/astro/test/ssr-prerender.test.js

@@ -37,6 +37,22 @@ describe('SSR: prerender', () => {
 			/** @type {Set<string>} */
 			const assets = app.manifest.assets;
 			assert.equal(assets.has('/static/index.html'), true);
+			const content = await fixture.readFile('/client/static/index.html');
+			const $ = cheerio.load(content);
+			assert.equal($('h1#greeting').text(), 'Hello world!');
+		});
+
+
+		it('skips prerendering for pages conflicting with a higher priority SSR route', async () => {
+			const app = await fixture.loadTestAdapterApp();
+			/** @type {Set<string>} */
+			const assets = app.manifest.assets;
+			assert.equal(assets.has('/non-prerendered/index.html'), false);
+
+			// Skipped page should not exist in the output
+			// test for bug where it was removed from the assets but stil rendered.
+			const generated = await fixture.readdir('/client');
+			assert.equal(generated.includes('not-prerendered'), false);
 		});
 	});