refactor: move page rendering in one single function (#7684)

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

@@ -2,25 +2,18 @@ import mime from 'mime';
 import type {
 	EndpointHandler,
 	ManifestData,
-	MiddlewareResponseHandler,
 	RouteData,
 	SSRElement,
 	SSRManifest,
 } from '../../@types/astro';
 import type { SinglePageBuiltModule } from '../build/types';
 import { attachToResponse, getSetCookiesFromResponse } from '../cookies/index.js';
-import { callEndpoint, createAPIContext } from '../endpoint/index.js';
+import { callEndpoint } from '../endpoint/index.js';
 import { consoleLogDestination } from '../logger/console.js';
 import { error, type LogOptions } from '../logger/core.js';
-import { callMiddleware } from '../middleware/callMiddleware.js';
 import { prependForwardSlash, removeTrailingForwardSlash } from '../path.js';
 import { RedirectSinglePageBuiltModule } from '../redirects/index.js';
-import {
+import { createEnvironment, createRenderContext, type Environment } from '../render/index.js';
-	createEnvironment,
-	createRenderContext,
-	renderPage,
-	type Environment,
-} from '../render/index.js';
 import { RouteCache } from '../render/route-cache.js';
 import {
 	createAssetLink,
@@ -29,6 +22,7 @@ import {
 } from '../render/ssr-element.js';
 import { matchRoute } from '../routing/match.js';
 import type { RouteInfo } from './types';
+import { tryRenderPage } from '../render/index.js';
 export { deserializeManifest } from './common.js';
 
 const clientLocalsSymbol = Symbol.for('astro.locals');
@@ -256,36 +250,8 @@ export class App {
 				env: this.#env,
 			});
 
-			const apiContext = createAPIContext({
+			const response = await tryRenderPage(renderContext, this.#env, mod, page.onRequest);
-				request: renderContext.request,
+
-				params: renderContext.params,
-				props: renderContext.props,
-				site: this.#env.site,
-				adapterName: this.#env.adapterName,
-			});
-			let response;
-			if (page.onRequest) {
-				response = await callMiddleware<Response>(
-					this.#env.logging,
-					page.onRequest as MiddlewareResponseHandler,
-					apiContext,
-					() => {
-						return renderPage({
-							mod,
-							renderContext,
-							env: this.#env,
-							cookies: apiContext.cookies,
-						});
-					}
-				);
-			} else {
-				response = await renderPage({
-					mod,
-					renderContext,
-					env: this.#env,
-					cookies: apiContext.cookies,
-				});
-			}
 			Reflect.set(request, responseSentSymbol, true);
 			return response;
 		} catch (err: any) {

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

@@ -12,7 +12,6 @@ import type {
 	GetStaticPathsItem,
 	ImageTransform,
 	MiddlewareHandler,
-	MiddlewareResponseHandler,
 	RouteData,
 	RouteType,
 	SSRError,
@@ -38,16 +37,15 @@ import {
 import { runHookBuildGenerated } from '../../integrations/index.js';
 import { isServerLikeOutput } from '../../prerender/utils.js';
 import { BEFORE_HYDRATION_SCRIPT_ID, PAGE_SCRIPT_ID } from '../../vite-plugin-scripts/index.js';
-import { callEndpoint, createAPIContext, throwIfRedirectNotAllowed } from '../endpoint/index.js';
+import { callEndpoint, throwIfRedirectNotAllowed } from '../endpoint/index.js';
 import { AstroError, AstroErrorData } from '../errors/index.js';
 import { debug, info } from '../logger/core.js';
-import { callMiddleware } from '../middleware/callMiddleware.js';
 import {
 	getRedirectLocationOrThrow,
 	RedirectSinglePageBuiltModule,
 	routeIsRedirect,
 } from '../redirects/index.js';
-import { createEnvironment, createRenderContext, renderPage } from '../render/index.js';
+import { createEnvironment, createRenderContext } from '../render/index.js';
 import { callGetStaticPaths } from '../render/route-cache.js';
 import {
 	createAssetLink,
@@ -71,6 +69,7 @@ import type {
 	StylesheetAsset,
 } from './types';
 import { getTimeStat } from './util.js';
+import { tryRenderPage } from '../render/index.js';
 
 function createEntryURL(filePath: string, outFolder: URL) {
 	return new URL('./' + filePath + `?time=${Date.now()}`, outFolder);
@@ -575,36 +574,7 @@ async function generatePath(
 	} else {
 		let response: Response;
 		try {
-			const apiContext = createAPIContext({
+			response = await tryRenderPage(renderContext, env, mod, onRequest);
-				request: renderContext.request,
-				params: renderContext.params,
-				props: renderContext.props,
-				site: env.site,
-				adapterName: env.adapterName,
-			});
-
-			if (onRequest) {
-				response = await callMiddleware<Response>(
-					env.logging,
-					onRequest as MiddlewareResponseHandler,
-					apiContext,
-					() => {
-						return renderPage({
-							mod,
-							renderContext,
-							env,
-							cookies: apiContext.cookies,
-						});
-					}
-				);
-			} else {
-				response = await renderPage({
-					mod,
-					renderContext,
-					env,
-					cookies: apiContext.cookies,
-				});
-			}
 		} catch (err) {
 			if (!AstroError.is(err) && !(err as SSRError).id && typeof err === 'object') {
 				(err as SSRError).id = pageData.component;

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

@@ -1,10 +1,17 @@
-import type { AstroCookies, ComponentInstance } from '../../@types/astro';
+import type {
+	AstroCookies,
+	ComponentInstance,
+	MiddlewareHandler,
+	MiddlewareResponseHandler,
+} from '../../@types/astro';
 import { renderPage as runtimeRenderPage } from '../../runtime/server/index.js';
 import { attachToResponse } from '../cookies/index.js';
 import { redirectRouteGenerate, redirectRouteStatus, routeIsRedirect } from '../redirects/index.js';
 import type { RenderContext } from './context.js';
 import type { Environment } from './environment.js';
 import { createResult } from './result.js';
+import { createAPIContext } from '../endpoint/index.js';
+import { callMiddleware } from '../middleware/callMiddleware.js';
 
 export type RenderPage = {
 	mod: ComponentInstance;
@@ -13,7 +20,7 @@ export type RenderPage = {
 	cookies: AstroCookies;
 };
 
-export async function renderPage({ mod, renderContext, env, cookies }: RenderPage) {
+async function renderPage({ mod, renderContext, env, cookies }: RenderPage) {
 	if (routeIsRedirect(renderContext.route)) {
 		return new Response(null, {
 			status: redirectRouteStatus(renderContext.route, renderContext.request.method),
@@ -72,3 +79,48 @@ export async function renderPage({ mod, renderContext, env, cookies }: RenderPag
 
 	return response;
 }
+
+/**
+ * It attempts to render a page.
+ *
+ * ## Errors
+ *
+ * It throws an error if the page can't be rendered.
+ */
+export async function tryRenderPage<MiddlewareReturnType = Response>(
+	renderContext: Readonly<RenderContext>,
+	env: Readonly<Environment>,
+	mod: Readonly<ComponentInstance>,
+	onRequest?: MiddlewareHandler<MiddlewareReturnType>
+): Promise<Response> {
+	const apiContext = createAPIContext({
+		request: renderContext.request,
+		params: renderContext.params,
+		props: renderContext.props,
+		site: env.site,
+		adapterName: env.adapterName,
+	});
+
+	if (onRequest) {
+		return await callMiddleware<Response>(
+			env.logging,
+			onRequest as MiddlewareResponseHandler,
+			apiContext,
+			() => {
+				return renderPage({
+					mod,
+					renderContext,
+					env,
+					cookies: apiContext.cookies,
+				});
+			}
+		);
+	} else {
+		return await renderPage({
+			mod,
+			renderContext,
+			env,
+			cookies: apiContext.cookies,
+		});
+	}
+}

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

@@ -6,12 +6,10 @@ import type {
 	SSRElement,
 } from '../../../@types/astro';
 import { PAGE_SCRIPT_ID } from '../../../vite-plugin-scripts/index.js';
-import { createAPIContext } from '../../endpoint/index.js';
 import { enhanceViteSSRError } from '../../errors/dev/index.js';
 import { AggregateError, CSSError, MarkdownError } from '../../errors/index.js';
-import { callMiddleware } from '../../middleware/callMiddleware.js';
 import { isPage, resolveIdToUrl, viteID } from '../../util.js';
-import { createRenderContext, loadRenderers, renderPage as coreRenderPage } from '../index.js';
+import { createRenderContext, loadRenderers, tryRenderPage } from '../index.js';
 import { getStylesForURL } from './css.js';
 import type { DevelopmentEnvironment } from './environment';
 import { getComponentMetadata } from './metadata.js';
@@ -164,31 +162,7 @@ export async function renderPage(options: SSROptions): Promise<Response> {
 		mod,
 		env,
 	});
-	const apiContext = createAPIContext({
+	const onRequest = options.middleware?.onRequest as MiddlewareResponseHandler | undefined;
-		request: options.request,
-		params: renderContext.params,
-		props: renderContext.props,
-		adapterName: options.env.adapterName,
-	});
-	if (options.middleware) {
-		if (options.middleware?.onRequest) {
-			const onRequest = options.middleware.onRequest as MiddlewareResponseHandler;
-			const response = await callMiddleware<Response>(env.logging, onRequest, apiContext, () => {
-				return coreRenderPage({
-					mod,
-					renderContext,
-					env: options.env,
-					cookies: apiContext.cookies,
-				});
-			});
 
-			return response;
+	return tryRenderPage(renderContext, env, mod, onRequest);
-		}
-	}
-	return await coreRenderPage({
-		mod,
-		renderContext,
-		env: options.env,
-		cookies: apiContext.cookies,
-	}); // NOTE: without "await", errors won’t get caught below
 }

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

@@ -1,6 +1,6 @@
 export { createRenderContext } from './context.js';
 export type { RenderContext } from './context.js';
-export { renderPage } from './core.js';
+export { tryRenderPage } from './core.js';
 export type { Environment } from './environment';
 export { createEnvironment } from './environment.js';
 export { getParamsAndProps } from './params-and-props.js';

packages/astro/test/units/render/head.test.js

@@ -9,7 +9,7 @@ import {
 	renderHead,
 	Fragment,
 } from '../../../dist/runtime/server/index.js';
-import { createRenderContext, renderPage } from '../../../dist/core/render/index.js';
+import { createRenderContext, tryRenderPage } from '../../../dist/core/render/index.js';
 import { createBasicEnvironment } from '../test-utils.js';
 import * as cheerio from 'cheerio';
 
@@ -96,13 +96,7 @@ describe('core/render', () => {
 				env,
 			});
 
-			const response = await renderPage({
+			const response = await tryRenderPage(ctx, env, PageModule);
-				mod: PageModule,
-				renderContext: ctx,
-				env,
-				params: ctx.params,
-				props: ctx.props,
-			});
 
 			const html = await response.text();
 			const $ = cheerio.load(html);
@@ -182,13 +176,7 @@ describe('core/render', () => {
 				mod: PageModule,
 			});
 
-			const response = await renderPage({
+			const response = await tryRenderPage(ctx, env, PageModule);
-				mod: PageModule,
-				renderContext: ctx,
-				env,
-				params: ctx.params,
-				props: ctx.props,
-			});
 			const html = await response.text();
 			const $ = cheerio.load(html);
 
@@ -234,13 +222,7 @@ describe('core/render', () => {
 				mod: PageModule,
 			});
 
-			const response = await renderPage({
+			const response = await tryRenderPage(ctx, env, PageModule);
-				mod: PageModule,
-				renderContext: ctx,
-				env,
-				params: ctx.params,
-				props: ctx.props,
-			});
 			const html = await response.text();
 			const $ = cheerio.load(html);
 

packages/astro/test/units/render/jsx.test.js

@@ -6,7 +6,11 @@ import {
 	renderSlot,
 } from '../../../dist/runtime/server/index.js';
 import { jsx } from '../../../dist/jsx-runtime/index.js';
-import { createRenderContext, renderPage, loadRenderer } from '../../../dist/core/render/index.js';
+import {
+	createRenderContext,
+	tryRenderPage,
+	loadRenderer,
+} from '../../../dist/core/render/index.js';
 import { createAstroJSXComponent, renderer as jsxRenderer } from '../../../dist/jsx/index.js';
 import { createBasicEnvironment } from '../test-utils.js';
 
@@ -46,11 +50,7 @@ describe('core/render', () => {
 				mod,
 			});
 
-			const response = await renderPage({
+			const response = await tryRenderPage(ctx, env, mod);
-				mod,
-				renderContext: ctx,
-				env,
-			});
 
 			expect(response.status).to.equal(200);
 
@@ -94,11 +94,7 @@ describe('core/render', () => {
 				env,
 				mod,
 			});
-			const response = await renderPage({
+			const response = await tryRenderPage(ctx, env, mod);
-				mod,
-				renderContext: ctx,
-				env,
-			});
 
 			expect(response.status).to.equal(200);
 
@@ -124,11 +120,7 @@ describe('core/render', () => {
 				mod,
 			});
 
-			const response = await renderPage({
+			const response = await tryRenderPage(ctx, env, mod);
-				mod,
-				renderContext: ctx,
-				env,
-			});
 
 			try {
 				await response.text();