Prerendering corner cases (#8070)

Co-authored-by: Nate Moore <natemoo-re@users.noreply.github.com>

.changeset/many-actors-flash.md

@@ -0,0 +1,5 @@
+---
+"astro": patch
+---
+
+Fix a handful of edge cases with prerendered 404/500 pages

packages/astro/src/core/app/index.ts

@@ -286,10 +286,16 @@ export class App {
 		const errorRouteData = matchRoute('/' + status, this.#manifestData);
 		const url = new URL(request.url);
 		if (errorRouteData) {
-			if (errorRouteData.prerender && !errorRouteData.route.endsWith(`/${status}`)) {
-				const statusURL = new URL(`${this.#baseWithoutTrailingSlash}/${status}`, url);
+			if (errorRouteData.prerender){
+				const maybeDotHtml = errorRouteData.route.endsWith(`/${status}`) ? '.html' : ''
+				const statusURL = new URL(`${this.#baseWithoutTrailingSlash}/${status}${maybeDotHtml}`, url);
 				const response = await fetch(statusURL.toString());
-				return this.#mergeResponses(response, originalResponse);
+
+				// response for /404.html and 500.html is 200, which is not meaningful
+				// so we create an override
+				const override = { status }
+
+				return this.#mergeResponses(response, originalResponse, override);
 			}
 			const mod = await this.#getModuleForRoute(errorRouteData);
 			try {
@@ -316,14 +322,30 @@ export class App {
 		return response;
 	}
 
-	#mergeResponses(newResponse: Response, oldResponse?: Response) {
-		if (!oldResponse) return newResponse;
-		const { status, statusText, headers } = oldResponse;
+	#mergeResponses(newResponse: Response, oldResponse?: Response, override?: { status : 404 | 500 }) {
+		if (!oldResponse) {
+			if (override !== undefined) {
+				return new Response(newResponse.body, {
+					status: override.status,
+					statusText: newResponse.statusText,
+					headers: newResponse.headers
+				})
+			}
+			return newResponse;
+		}
+
+		const { statusText, headers } = oldResponse;
+
+		// If the the new response did not have a meaningful status, an override may have been provided
+		// If the original status was 200 (default), override it with the new status (probably 404 or 500)
+		// Otherwise, the user set a specific status while rendering and we should respect that one
+		const status = 
+				override?.status ? override.status :
+				oldResponse.status === 200 ? newResponse.status :
+				oldResponse.status
 
 		return new Response(newResponse.body, {
-			// If the original status was 200 (default), override it with the new status (probably 404 or 500)
-			// Otherwise, the user set a specific status while rendering and we should respect that one
-			status: status === 200 ? newResponse.status : status,
+			status,
 			statusText: status === 200 ? newResponse.statusText : statusText,
 			headers: new Headers(Array.from(headers)),
 		});

packages/astro/src/core/build/static-build.ts

@@ -308,8 +308,12 @@ async function runPostBuildHooks(
 async function cleanStaticOutput(opts: StaticBuildOptions, internals: BuildInternals) {
 	const allStaticFiles = new Set();
 	for (const pageData of eachPageData(internals)) {
-		if (pageData.route.prerender)
-			allStaticFiles.add(internals.pageToBundleMap.get(pageData.moduleSpecifier));
+		if (pageData.route.prerender) {
+			const { moduleSpecifier } = pageData;
+			const pageBundleId = internals.pageToBundleMap.get(moduleSpecifier);
+			const entryBundleId = internals.entrySpecifierToBundleMap.get(moduleSpecifier);
+			allStaticFiles.add(pageBundleId ?? entryBundleId);
+		}
 	}
 	const ssr = isServerLikeOutput(opts.settings.config);
 	const out = ssr
@@ -337,7 +341,8 @@ async function cleanStaticOutput(opts: StaticBuildOptions, internals: BuildInter
 				// Replace exports (only prerendered pages) with a noop
 				let value = 'const noop = () => {};';
 				for (const e of exports) {
-					value += `\nexport const ${e.n} = noop;`;
+					if (e.n === 'default') value += `\n export default noop;`
+					else value += `\nexport const ${e.n} = noop;`;
 				}
 				await fs.promises.writeFile(url, value, { encoding: 'utf8' });
 			})

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

@@ -30,8 +30,7 @@ describe('SSR: prerender', () => {
 			const app = await fixture.loadTestAdapterApp();
 			/** @type {Set<string>} */
 			const assets = app.manifest.assets;
-			expect(assets.size).to.equal(1);
-			expect(Array.from(assets)[0].endsWith('static/index.html')).to.be.true;
+			expect(assets).to.contain('/static/index.html');
 		});
 	});
 

packages/integrations/node/test/fixtures/prerender-404/package.json → packages/integrations/node/test/fixtures/prerender-404-500/package.json

@@ -1,7 +1,8 @@
 {
-  "name": "@test/nodejs-prerender-404",
+  "name": "@test/nodejs-prerender-404-500",
   "version": "0.0.0",
   "private": true,
+  "type": "module",
   "dependencies": {
     "astro": "workspace:*",
     "@astrojs/node": "workspace:*"

packages/integrations/node/test/fixtures/prerender-404-500/src/external-stylesheet.css

@@ -0,0 +1,3 @@
+body {
+    background-color: ivory;
+}

packages/integrations/node/test/fixtures/prerender-404-500/src/nondeterminism-404.ts

@@ -0,0 +1,17 @@
+// This module is only used by the prerendered 404.astro.
+// It exhibits different behavior if it's called more than once,
+// which is detected by a test and interpreted as a failure.
+
+let usedOnce = false
+let dynamicMessage = "Page was not prerendered"
+
+export default function () {
+    if (usedOnce === false) {
+        usedOnce = true
+        return "Page does not exist"
+    }
+
+    dynamicMessage += "+"
+
+    return dynamicMessage
+}

packages/integrations/node/test/fixtures/prerender-404-500/src/nondeterminism-500.ts

@@ -0,0 +1,17 @@
+// This module is only used by the prerendered 500.astro.
+// It exhibits different behavior if it's called more than once,
+// which is detected by a test and interpreted as a failure.
+
+let usedOnce = false
+let dynamicMessage = "Page was not prerendered"
+
+export default function () {
+    if (usedOnce === false) {
+        usedOnce = true
+        return "Something went wrong"
+    }
+
+    dynamicMessage += "+"
+
+    return dynamicMessage
+}

packages/integrations/node/test/fixtures/prerender-404-500/src/pages/404.astro

@@ -0,0 +1,5 @@
+---
+import message from "../nondeterminism-404"
+export const prerender = true;
+---
+{message()}

packages/integrations/node/test/fixtures/prerender-404-500/src/pages/500.astro

@@ -0,0 +1,6 @@
+---
+import "../external-stylesheet.css"
+import message from "../nondeterminism-500"
+export const prerender = true
+---
+<h1>{message()}</h1>

packages/integrations/node/test/fixtures/prerender-404-500/src/pages/fivehundred.astro

@@ -0,0 +1,4 @@
+---
+return new Response(null, { status: 500 })
+---
+<p>This html will not be served</p>

packages/integrations/node/test/image.test.js

@@ -32,9 +32,6 @@ describe('Image endpoint', () => {
 			'/_image?href=/_astro/some_penguin.97ef5f92.png&w=50&f=webp'
 		);
 
-		console.log(resImage);
-		const content = resImage.text();
-		console.log(content);
 		expect(resImage.status).to.equal(200);
 	});
 });