From ed20154a5c89f07e87784c5fe80a028ceef741fa Mon Sep 17 00:00:00 2001 From: Emanuele Stoppa Date: Mon, 17 Jul 2023 17:22:53 +0100 Subject: [PATCH] refactor: move page rendering in one single function (#7684) --- packages/astro/src/core/app/index.ts | 44 ++------------- packages/astro/src/core/build/generate.ts | 38 ++----------- packages/astro/src/core/render/core.ts | 56 ++++++++++++++++++- packages/astro/src/core/render/dev/index.ts | 32 +---------- packages/astro/src/core/render/index.ts | 2 +- packages/astro/test/units/render/head.test.js | 26 ++------- packages/astro/test/units/render/jsx.test.js | 24 +++----- 7 files changed, 79 insertions(+), 143 deletions(-) diff --git a/packages/astro/src/core/app/index.ts b/packages/astro/src/core/app/index.ts index b49a4728a..b50ab123f 100644 --- a/packages/astro/src/core/app/index.ts +++ b/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 { - createEnvironment, - createRenderContext, - renderPage, - type Environment, -} from '../render/index.js'; +import { createEnvironment, createRenderContext, 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({ - 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( - 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, - }); - } + const response = await tryRenderPage(renderContext, this.#env, mod, page.onRequest); + Reflect.set(request, responseSentSymbol, true); return response; } catch (err: any) { diff --git a/packages/astro/src/core/build/generate.ts b/packages/astro/src/core/build/generate.ts index 2bbc20c47..72cc23f55 100644 --- a/packages/astro/src/core/build/generate.ts +++ b/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({ - request: renderContext.request, - params: renderContext.params, - props: renderContext.props, - site: env.site, - adapterName: env.adapterName, - }); - - if (onRequest) { - response = await callMiddleware( - env.logging, - onRequest as MiddlewareResponseHandler, - apiContext, - () => { - return renderPage({ - mod, - renderContext, - env, - cookies: apiContext.cookies, - }); - } - ); - } else { - response = await renderPage({ - mod, - renderContext, - env, - cookies: apiContext.cookies, - }); - } + response = await tryRenderPage(renderContext, env, mod, onRequest); } catch (err) { if (!AstroError.is(err) && !(err as SSRError).id && typeof err === 'object') { (err as SSRError).id = pageData.component; diff --git a/packages/astro/src/core/render/core.ts b/packages/astro/src/core/render/core.ts index 25834357e..b268bc890 100644 --- a/packages/astro/src/core/render/core.ts +++ b/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( + renderContext: Readonly, + env: Readonly, + mod: Readonly, + onRequest?: MiddlewareHandler +): Promise { + const apiContext = createAPIContext({ + request: renderContext.request, + params: renderContext.params, + props: renderContext.props, + site: env.site, + adapterName: env.adapterName, + }); + + if (onRequest) { + return await callMiddleware( + env.logging, + onRequest as MiddlewareResponseHandler, + apiContext, + () => { + return renderPage({ + mod, + renderContext, + env, + cookies: apiContext.cookies, + }); + } + ); + } else { + return await renderPage({ + mod, + renderContext, + env, + cookies: apiContext.cookies, + }); + } +} diff --git a/packages/astro/src/core/render/dev/index.ts b/packages/astro/src/core/render/dev/index.ts index 360832af4..97b3a0c33 100644 --- a/packages/astro/src/core/render/dev/index.ts +++ b/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 { mod, env, }); - const apiContext = createAPIContext({ - 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(env.logging, onRequest, apiContext, () => { - return coreRenderPage({ - mod, - renderContext, - env: options.env, - cookies: apiContext.cookies, - }); - }); + const onRequest = options.middleware?.onRequest as MiddlewareResponseHandler | undefined; - return response; - } - } - return await coreRenderPage({ - mod, - renderContext, - env: options.env, - cookies: apiContext.cookies, - }); // NOTE: without "await", errors won’t get caught below + return tryRenderPage(renderContext, env, mod, onRequest); } diff --git a/packages/astro/src/core/render/index.ts b/packages/astro/src/core/render/index.ts index 9eb85f4d0..410998d01 100644 --- a/packages/astro/src/core/render/index.ts +++ b/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'; diff --git a/packages/astro/test/units/render/head.test.js b/packages/astro/test/units/render/head.test.js index ef6ef0b92..da4863fb1 100644 --- a/packages/astro/test/units/render/head.test.js +++ b/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({ - mod: PageModule, - renderContext: ctx, - env, - params: ctx.params, - props: ctx.props, - }); + const response = await tryRenderPage(ctx, env, PageModule); const html = await response.text(); const $ = cheerio.load(html); @@ -182,13 +176,7 @@ describe('core/render', () => { mod: PageModule, }); - const response = await renderPage({ - mod: PageModule, - renderContext: ctx, - env, - params: ctx.params, - props: ctx.props, - }); + const response = await tryRenderPage(ctx, env, PageModule); const html = await response.text(); const $ = cheerio.load(html); @@ -234,13 +222,7 @@ describe('core/render', () => { mod: PageModule, }); - const response = await renderPage({ - mod: PageModule, - renderContext: ctx, - env, - params: ctx.params, - props: ctx.props, - }); + const response = await tryRenderPage(ctx, env, PageModule); const html = await response.text(); const $ = cheerio.load(html); diff --git a/packages/astro/test/units/render/jsx.test.js b/packages/astro/test/units/render/jsx.test.js index 46058cbff..61e5ee803 100644 --- a/packages/astro/test/units/render/jsx.test.js +++ b/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({ - mod, - renderContext: ctx, - env, - }); + const response = await tryRenderPage(ctx, env, mod); expect(response.status).to.equal(200); @@ -94,11 +94,7 @@ describe('core/render', () => { env, mod, }); - const response = await renderPage({ - mod, - renderContext: ctx, - env, - }); + const response = await tryRenderPage(ctx, env, mod); expect(response.status).to.equal(200); @@ -124,11 +120,7 @@ describe('core/render', () => { mod, }); - const response = await renderPage({ - mod, - renderContext: ctx, - env, - }); + const response = await tryRenderPage(ctx, env, mod); try { await response.text();