From 1568cb43244698bed10a436153443a9073d7b45f Mon Sep 17 00:00:00 2001 From: Emanuele Stoppa Date: Fri, 14 Jul 2023 15:58:35 +0100 Subject: [PATCH] refactor: use SSR manifest in dev (#7587) --- packages/astro/src/core/app/index.ts | 46 ++++++++---- packages/astro/src/core/build/generate.ts | 15 ++-- .../astro/src/core/render/dev/environment.ts | 14 ++-- .../src/vite-plugin-astro-server/plugin.ts | 54 ++++++++++--- .../src/vite-plugin-astro-server/request.ts | 53 +++++++------ .../src/vite-plugin-astro-server/route.ts | 75 +++++++++++-------- .../test/units/routing/route-matching.test.js | 12 +-- .../vite-plugin-astro-server/request.test.js | 10 ++- 8 files changed, 182 insertions(+), 97 deletions(-) diff --git a/packages/astro/src/core/app/index.ts b/packages/astro/src/core/app/index.ts index 3b82fa9833..b49a4728aa 100644 --- a/packages/astro/src/core/app/index.ts +++ b/packages/astro/src/core/app/index.ts @@ -40,6 +40,9 @@ export interface MatchOptions { } export class App { + /** + * The current environment of the application + */ #env: Environment; #manifest: SSRManifest; #manifestData: ManifestData; @@ -49,7 +52,6 @@ export class App { dest: consoleLogDestination, level: 'info', }; - #base: string; #baseWithoutTrailingSlash: string; constructor(manifest: SSRManifest, streaming = true) { @@ -58,26 +60,41 @@ export class App { routes: manifest.routes.map((route) => route.routeData), }; this.#routeDataToRouteInfo = new Map(manifest.routes.map((route) => [route.routeData, route])); - this.#env = createEnvironment({ - adapterName: manifest.adapterName, + this.#baseWithoutTrailingSlash = removeTrailingForwardSlash(this.#manifest.base); + this.#env = this.#createEnvironment(streaming); + } + + set setManifest(newManifest: SSRManifest) { + this.#manifest = newManifest; + } + + /** + * Creates an environment by reading the stored manifest + * + * @param streaming + * @private + */ + #createEnvironment(streaming = false) { + return createEnvironment({ + adapterName: this.#manifest.adapterName, logging: this.#logging, - markdown: manifest.markdown, + markdown: this.#manifest.markdown, mode: 'production', - compressHTML: manifest.compressHTML, - renderers: manifest.renderers, - clientDirectives: manifest.clientDirectives, - async resolve(specifier: string) { - if (!(specifier in manifest.entryModules)) { + compressHTML: this.#manifest.compressHTML, + renderers: this.#manifest.renderers, + clientDirectives: this.#manifest.clientDirectives, + resolve: async (specifier: string) => { + if (!(specifier in this.#manifest.entryModules)) { throw new Error(`Unable to resolve [${specifier}]`); } - const bundlePath = manifest.entryModules[specifier]; + const bundlePath = this.#manifest.entryModules[specifier]; switch (true) { case bundlePath.startsWith('data:'): case bundlePath.length === 0: { return bundlePath; } default: { - return createAssetLink(bundlePath, manifest.base, manifest.assetsPrefix); + return createAssetLink(bundlePath, this.#manifest.base, this.#manifest.assetsPrefix); } } }, @@ -86,12 +103,13 @@ export class App { ssr: true, streaming, }); + } - this.#base = this.#manifest.base || '/'; - this.#baseWithoutTrailingSlash = removeTrailingForwardSlash(this.#base); + set setManifestData(newManifestData: ManifestData) { + this.#manifestData = newManifestData; } removeBase(pathname: string) { - if (pathname.startsWith(this.#base)) { + if (pathname.startsWith(this.#manifest.base)) { return pathname.slice(this.#baseWithoutTrailingSlash.length + 1); } return pathname; diff --git a/packages/astro/src/core/build/generate.ts b/packages/astro/src/core/build/generate.ts index 501d8ff26a..2bbc20c47d 100644 --- a/packages/astro/src/core/build/generate.ts +++ b/packages/astro/src/core/build/generate.ts @@ -163,27 +163,27 @@ export async function generatePages(opts: StaticBuildOptions, internals: BuildIn } } else { const ssrEntry = ssrEntryPage as SinglePageBuiltModule; - const manifest = generateRuntimeManifest(opts.settings, internals, ssrEntry.renderers); + const manifest = createBuildManifest(opts.settings, internals, ssrEntry.renderers); await generatePage(opts, internals, pageData, ssrEntry, builtPaths, manifest); } } } for (const pageData of eachRedirectPageData(internals)) { const entry = await getEntryForRedirectRoute(pageData.route, internals, outFolder); - const manifest = generateRuntimeManifest(opts.settings, internals, entry.renderers); + const manifest = createBuildManifest(opts.settings, internals, entry.renderers); await generatePage(opts, internals, pageData, entry, builtPaths, manifest); } } else { for (const [pageData, filePath] of eachPageDataFromEntryPoint(internals)) { const ssrEntryURLPage = createEntryURL(filePath, outFolder); const entry: SinglePageBuiltModule = await import(ssrEntryURLPage.toString()); - const manifest = generateRuntimeManifest(opts.settings, internals, entry.renderers); + const manifest = createBuildManifest(opts.settings, internals, entry.renderers); await generatePage(opts, internals, pageData, entry, builtPaths, manifest); } for (const pageData of eachRedirectPageData(internals)) { const entry = await getEntryForRedirectRoute(pageData.route, internals, outFolder); - const manifest = generateRuntimeManifest(opts.settings, internals, entry.renderers); + const manifest = createBuildManifest(opts.settings, internals, entry.renderers); await generatePage(opts, internals, pageData, entry, builtPaths, manifest); } } @@ -521,8 +521,7 @@ async function generatePath( clientDirectives: manifest.clientDirectives, compressHTML: manifest.compressHTML, async resolve(specifier: string) { - // NOTE: next PR, borrow logic from build manifest maybe? - const hashedFilePath = internals.entrySpecifierToBundleMap.get(specifier); + const hashedFilePath = manifest.entryModules[specifier]; if (typeof hashedFilePath !== 'string') { // If no "astro:scripts/before-hydration.js" script exists in the build, // then we can assume that no before-hydration scripts are needed. @@ -656,14 +655,14 @@ async function generatePath( * @param settings * @param renderers */ -export function generateRuntimeManifest( +export function createBuildManifest( settings: AstroSettings, internals: BuildInternals, renderers: SSRLoadedRenderer[] ): SSRManifest { return { assets: new Set(), - entryModules: {}, + entryModules: Object.fromEntries(internals.entrySpecifierToBundleMap.entries()), routes: [], adapterName: '', markdown: settings.config.markdown, diff --git a/packages/astro/src/core/render/dev/environment.ts b/packages/astro/src/core/render/dev/environment.ts index e97b5df083..6d797b5218 100644 --- a/packages/astro/src/core/render/dev/environment.ts +++ b/packages/astro/src/core/render/dev/environment.ts @@ -1,9 +1,8 @@ -import type { AstroSettings, RuntimeMode } from '../../../@types/astro'; +import type { AstroSettings, RuntimeMode, SSRManifest } from '../../../@types/astro'; import { isServerLikeOutput } from '../../../prerender/utils.js'; import type { LogOptions } from '../../logger/core.js'; import type { ModuleLoader } from '../../module-loader/index'; import type { Environment } from '../index'; - import { createEnvironment } from '../index.js'; import { RouteCache } from '../route-cache.js'; import { createResolve } from './resolve.js'; @@ -14,23 +13,24 @@ export type DevelopmentEnvironment = Environment & { }; export function createDevelopmentEnvironment( + manifest: SSRManifest, settings: AstroSettings, logging: LogOptions, loader: ModuleLoader ): DevelopmentEnvironment { const mode: RuntimeMode = 'development'; let env = createEnvironment({ - adapterName: settings.adapter?.name, + adapterName: manifest.adapterName, logging, - markdown: settings.config.markdown, + markdown: manifest.markdown, mode, // This will be overridden in the dev server renderers: [], - clientDirectives: settings.clientDirectives, - compressHTML: settings.config.compressHTML, + clientDirectives: manifest.clientDirectives, + compressHTML: manifest.compressHTML, resolve: createResolve(loader, settings.config.root), routeCache: new RouteCache(logging, mode), - site: settings.config.site, + site: manifest.site, ssr: isServerLikeOutput(settings.config), streaming: true, }); diff --git a/packages/astro/src/vite-plugin-astro-server/plugin.ts b/packages/astro/src/vite-plugin-astro-server/plugin.ts index 72d6deb957..65c565678b 100644 --- a/packages/astro/src/vite-plugin-astro-server/plugin.ts +++ b/packages/astro/src/vite-plugin-astro-server/plugin.ts @@ -1,6 +1,5 @@ import type * as vite from 'vite'; import type { AstroSettings, ManifestData } from '../@types/astro'; - import type fs from 'fs'; import { patchOverlay } from '../core/errors/overlay.js'; import type { LogOptions } from '../core/logger/core.js'; @@ -10,6 +9,7 @@ import { createRouteManifest } from '../core/routing/index.js'; import { baseMiddleware } from './base.js'; import { createController } from './controller.js'; import { handleRequest } from './request.js'; +import type { SSRManifest } from '../@types/astro'; export interface AstroPluginOptions { settings: AstroSettings; @@ -26,15 +26,16 @@ export default function createVitePluginAstroServer({ name: 'astro:server', configureServer(viteServer) { const loader = createViteLoader(viteServer); - let env = createDevelopmentEnvironment(settings, logging, loader); - let manifest: ManifestData = createRouteManifest({ settings, fsMod }, logging); - const serverController = createController({ loader }); + const manifest = createDevelopmentManifest(settings); + const env = createDevelopmentEnvironment(manifest, settings, logging, loader); + let manifestData: ManifestData = createRouteManifest({ settings, fsMod }, logging); + const controller = createController({ loader }); /** rebuild the route cache + manifest, as needed. */ function rebuildManifest(needsManifestRebuild: boolean) { env.routeCache.clearAll(); if (needsManifestRebuild) { - manifest = createRouteManifest({ settings }, logging); + manifestData = createRouteManifest({ settings }, logging); } } // Rebuild route manifest on file change, if needed. @@ -51,13 +52,20 @@ export default function createVitePluginAstroServer({ }); } // Note that this function has a name so other middleware can find it. - viteServer.middlewares.use(async function astroDevHandler(req, res) { - if (req.url === undefined || !req.method) { - res.writeHead(500, 'Incomplete request'); - res.end(); + viteServer.middlewares.use(async function astroDevHandler(request, response) { + if (request.url === undefined || !request.method) { + response.writeHead(500, 'Incomplete request'); + response.end(); return; } - handleRequest(env, manifest, serverController, req, res); + handleRequest({ + env, + manifestData, + controller, + incomingRequest: request, + incomingResponse: response, + manifest, + }); }); }; }, @@ -70,3 +78,29 @@ export default function createVitePluginAstroServer({ }, }; } + +/** + * It creates a `SSRManifest` from the `AstroSettings`. + * + * Renderers needs to be pulled out from the page module emitted during the build. + * @param settings + * @param renderers + */ +export function createDevelopmentManifest(settings: AstroSettings): SSRManifest { + return { + compressHTML: settings.config.compressHTML, + assets: new Set(), + entryModules: {}, + routes: [], + adapterName: '', + markdown: settings.config.markdown, + clientDirectives: settings.clientDirectives, + renderers: [], + base: settings.config.base, + assetsPrefix: settings.config.build.assetsPrefix, + site: settings.config.site + ? new URL(settings.config.base, settings.config.site).toString() + : settings.config.site, + componentMetadata: new Map(), + }; +} diff --git a/packages/astro/src/vite-plugin-astro-server/request.ts b/packages/astro/src/vite-plugin-astro-server/request.ts index 21599064c7..df9655c75a 100644 --- a/packages/astro/src/vite-plugin-astro-server/request.ts +++ b/packages/astro/src/vite-plugin-astro-server/request.ts @@ -1,5 +1,5 @@ import type http from 'http'; -import type { ManifestData } from '../@types/astro'; +import type { ManifestData, SSRManifest } from '../@types/astro'; import type { DevelopmentEnvironment } from '../core/render/dev/index'; import type { DevServerController } from './controller'; @@ -14,22 +14,32 @@ import { runWithErrorHandling } from './controller.js'; import { handle500Response } from './response.js'; import { handleRoute, matchRoute } from './route.js'; +type HandleRequest = { + env: DevelopmentEnvironment; + manifestData: ManifestData; + controller: DevServerController; + incomingRequest: http.IncomingMessage; + incomingResponse: http.ServerResponse; + manifest: SSRManifest; +}; + /** The main logic to route dev server requests to pages in Astro. */ -export async function handleRequest( - env: DevelopmentEnvironment, - manifest: ManifestData, - controller: DevServerController, - req: http.IncomingMessage, - res: http.ServerResponse -) { +export async function handleRequest({ + env, + manifestData, + controller, + incomingRequest, + incomingResponse, + manifest, +}: HandleRequest) { const { settings, loader: moduleLoader } = env; const { config } = settings; - const origin = `${moduleLoader.isHttps() ? 'https' : 'http'}://${req.headers.host}`; + const origin = `${moduleLoader.isHttps() ? 'https' : 'http'}://${incomingRequest.headers.host}`; const buildingToSSR = isServerLikeOutput(config); - const url = new URL(origin + req.url); + const url = new URL(origin + incomingRequest.url); let pathname: string; - if (config.trailingSlash === 'never' && !req.url) { + if (config.trailingSlash === 'never' && !incomingRequest.url) { pathname = ''; } else { pathname = decodeURI(url.pathname); @@ -50,13 +60,13 @@ export async function handleRequest( } let body: ArrayBuffer | undefined = undefined; - if (!(req.method === 'GET' || req.method === 'HEAD')) { + if (!(incomingRequest.method === 'GET' || incomingRequest.method === 'HEAD')) { let bytes: Uint8Array[] = []; await new Promise((resolve) => { - req.on('data', (part) => { + incomingRequest.on('data', (part) => { bytes.push(part); }); - req.on('end', resolve); + incomingRequest.on('end', resolve); }); body = Buffer.concat(bytes); } @@ -65,19 +75,20 @@ export async function handleRequest( controller, pathname, async run() { - const matchedRoute = await matchRoute(pathname, env, manifest); + const matchedRoute = await matchRoute(pathname, env, manifestData); const resolvedPathname = matchedRoute?.resolvedPathname ?? pathname; - return await handleRoute( + return await handleRoute({ matchedRoute, url, - resolvedPathname, + pathname: resolvedPathname, body, origin, env, + manifestData, + incomingRequest: incomingRequest, + incomingResponse: incomingResponse, manifest, - req, - res - ); + }); }, onError(_err) { const err = createSafeError(_err); @@ -94,7 +105,7 @@ export async function handleRequest( telemetry.record(eventError({ cmd: 'dev', err: errorWithMetadata, isFatal: false })); error(env.logging, null, msg.formatErrorMessage(errorWithMetadata)); - handle500Response(moduleLoader, res, errorWithMetadata); + handle500Response(moduleLoader, incomingResponse, errorWithMetadata); return err; }, diff --git a/packages/astro/src/vite-plugin-astro-server/route.ts b/packages/astro/src/vite-plugin-astro-server/route.ts index 321e1edac2..388ce77434 100644 --- a/packages/astro/src/vite-plugin-astro-server/route.ts +++ b/packages/astro/src/vite-plugin-astro-server/route.ts @@ -1,6 +1,6 @@ import type http from 'http'; import mime from 'mime'; -import type { ComponentInstance, ManifestData, RouteData } from '../@types/astro'; +import type { ComponentInstance, ManifestData, RouteData, SSRManifest } from '../@types/astro'; import { attachToResponse } from '../core/cookies/index.js'; import { call as callEndpoint } from '../core/endpoint/dev/index.js'; import { throwIfRedirectNotAllowed } from '../core/endpoint/index.js'; @@ -114,25 +114,39 @@ export async function matchRoute( return undefined; } -export async function handleRoute( - matchedRoute: AsyncReturnType, - url: URL, - pathname: string, - body: ArrayBuffer | undefined, - origin: string, - env: DevelopmentEnvironment, - manifest: ManifestData, - req: http.IncomingMessage, - res: http.ServerResponse -): Promise { +type HandleRoute = { + matchedRoute: AsyncReturnType; + url: URL; + pathname: string; + body: ArrayBuffer | undefined; + origin: string; + env: DevelopmentEnvironment; + manifestData: ManifestData; + incomingRequest: http.IncomingMessage; + incomingResponse: http.ServerResponse; + manifest: SSRManifest; +}; + +export async function handleRoute({ + matchedRoute, + url, + pathname, + body, + origin, + env, + manifestData, + incomingRequest, + incomingResponse, + manifest, +}: HandleRoute): Promise { const { logging, settings } = env; if (!matchedRoute) { - return handle404Response(origin, req, res); + return handle404Response(origin, incomingRequest, incomingResponse); } if (matchedRoute.route.type === 'redirect' && !settings.config.experimental.redirects) { writeWebResponse( - res, + incomingResponse, new Response(`To enable redirect set experimental.redirects to \`true\`.`, { status: 400, }) @@ -148,18 +162,18 @@ export async function handleRoute( // Headers are only available when using SSR. const request = createRequest({ url, - headers: buildingToSSR ? req.headers : new Headers(), - method: req.method, + headers: buildingToSSR ? incomingRequest.headers : new Headers(), + method: incomingRequest.method, body, logging, ssr: buildingToSSR, - clientAddress: buildingToSSR ? req.socket.remoteAddress : undefined, - locals: Reflect.get(req, clientLocalsSymbol), // Allows adapters to pass in locals in dev mode. + clientAddress: buildingToSSR ? incomingRequest.socket.remoteAddress : undefined, + locals: Reflect.get(incomingRequest, clientLocalsSymbol), // Allows adapters to pass in locals in dev mode. }); // Set user specified headers to response object. for (const [name, value] of Object.entries(config.server.headers ?? {})) { - if (value) res.setHeader(name, value); + if (value) incomingResponse.setHeader(name, value); } const options: SSROptions = { @@ -179,21 +193,22 @@ export async function handleRoute( const result = await callEndpoint(options); if (result.type === 'response') { if (result.response.headers.get('X-Astro-Response') === 'Not-Found') { - const fourOhFourRoute = await matchRoute('/404', env, manifest); - return handleRoute( - fourOhFourRoute, - new URL('/404', url), - '/404', + const fourOhFourRoute = await matchRoute('/404', env, manifestData); + return handleRoute({ + matchedRoute: fourOhFourRoute, + url: new URL('/404', url), + pathname: '/404', body, origin, env, + manifestData, + incomingRequest, + incomingResponse, manifest, - req, - res - ); + }); } throwIfRedirectNotAllowed(result.response, config); - await writeWebResponse(res, result.response); + await writeWebResponse(incomingResponse, 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') @@ -211,11 +226,11 @@ export async function handleRoute( }, }); attachToResponse(response, result.cookies); - await writeWebResponse(res, response); + await writeWebResponse(incomingResponse, response); } } else { const result = await renderPage(options); throwIfRedirectNotAllowed(result, config); - return await writeSSRResult(request, result, res); + return await writeSSRResult(request, result, incomingResponse); } } diff --git a/packages/astro/test/units/routing/route-matching.test.js b/packages/astro/test/units/routing/route-matching.test.js index 1c4e178d8b..0cf40f28fc 100644 --- a/packages/astro/test/units/routing/route-matching.test.js +++ b/packages/astro/test/units/routing/route-matching.test.js @@ -9,6 +9,7 @@ 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'; +import { createDevelopmentManifest } from '../../../dist/vite-plugin-astro-server/plugin.js'; const root = new URL('../../fixtures/alias/', import.meta.url); const fileSystem = { @@ -120,7 +121,7 @@ const fileSystem = { describe('Route matching', () => { let env; - let manifest; + let manifestData; let container; let settings; @@ -138,8 +139,9 @@ describe('Route matching', () => { settings = container.settings; const loader = createViteLoader(container.viteServer); - env = createDevelopmentEnvironment(container.settings, defaultLogging, loader); - manifest = createRouteManifest( + const manifest = createDevelopmentManifest(container.settings); + env = createDevelopmentEnvironment(manifest, container.settings, defaultLogging, loader); + manifestData = createRouteManifest( { cwd: fileURLToPath(root), settings, @@ -155,7 +157,7 @@ describe('Route matching', () => { describe('Matched routes', () => { it('should be sorted correctly', async () => { - const matches = matchAllRoutes('/try-matching-a-route', manifest); + const matches = matchAllRoutes('/try-matching-a-route', manifestData); const preloadedMatches = await getSortedPreloadedMatches({ env, matches, settings }); const sortedRouteNames = preloadedMatches.map((match) => match.route.route); @@ -169,7 +171,7 @@ describe('Route matching', () => { ]); }); it('nested should be sorted correctly', async () => { - const matches = matchAllRoutes('/nested/try-matching-a-route', manifest); + const matches = matchAllRoutes('/nested/try-matching-a-route', manifestData); const preloadedMatches = await getSortedPreloadedMatches({ env, matches, settings }); const sortedRouteNames = preloadedMatches.map((match) => match.route.route); diff --git a/packages/astro/test/units/vite-plugin-astro-server/request.test.js b/packages/astro/test/units/vite-plugin-astro-server/request.test.js index 67d99c1776..fba0a49ab6 100644 --- a/packages/astro/test/units/vite-plugin-astro-server/request.test.js +++ b/packages/astro/test/units/vite-plugin-astro-server/request.test.js @@ -44,7 +44,7 @@ describe('vite-plugin-astro-server', () => { }, '/' ); - const manifest = createRouteManifest( + const manifestData = createRouteManifest( { fsMod: fs, settings: env.settings, @@ -53,7 +53,13 @@ describe('vite-plugin-astro-server', () => { ); try { - await handleRequest(env, manifest, controller, req, res); + await handleRequest({ + env, + manifestData, + controller, + incomingRequest: req, + incomingResponse: res, + }); } catch (err) { expect(err.message).to.be.undefined(); }