From d4a6ab7339043042fd62dffd30ba078edae55f86 Mon Sep 17 00:00:00 2001 From: Emanuele Stoppa Date: Wed, 30 Aug 2023 18:56:10 +0100 Subject: [PATCH] fix(ssr): inline middleware during the build, not rely on file system (#8300) * fix(ssr): inline middleware during the build, not rely on file system * feedback * chore: fix lint --------- Co-authored-by: Nate Moore Co-authored-by: Nate Moore --- .changeset/large-bugs-matter.md | 5 ++++ packages/astro/src/core/app/index.ts | 24 ++++++------------- packages/astro/src/core/app/types.ts | 1 - .../astro/src/core/build/buildPipeline.ts | 7 ------ packages/astro/src/core/build/generate.ts | 11 +++++---- .../src/core/build/plugins/plugin-manifest.ts | 6 ----- packages/astro/src/core/build/types.ts | 2 ++ .../astro/src/core/redirects/component.ts | 1 + .../src/vite-plugin-astro-server/plugin.ts | 1 - .../vercel/test/edge-middleware.test.js | 2 +- 10 files changed, 22 insertions(+), 38 deletions(-) create mode 100644 .changeset/large-bugs-matter.md diff --git a/.changeset/large-bugs-matter.md b/.changeset/large-bugs-matter.md new file mode 100644 index 000000000..f2125f09d --- /dev/null +++ b/.changeset/large-bugs-matter.md @@ -0,0 +1,5 @@ +--- +'astro': patch +--- + +Correctly retrive middleware when using it in SSR enviroments. diff --git a/packages/astro/src/core/app/index.ts b/packages/astro/src/core/app/index.ts index 71a8da439..a3af4bcdd 100644 --- a/packages/astro/src/core/app/index.ts +++ b/packages/astro/src/core/app/index.ts @@ -56,8 +56,6 @@ export class App { }); #baseWithoutTrailingSlash: string; #pipeline: SSRRoutePipeline; - #onRequest: MiddlewareEndpointHandler | undefined; - #middlewareLoaded: boolean; #adapterLogger: AstroIntegrationLogger; constructor(manifest: SSRManifest, streaming = true) { @@ -68,7 +66,6 @@ export class App { this.#routeDataToRouteInfo = new Map(manifest.routes.map((route) => [route.routeData, route])); this.#baseWithoutTrailingSlash = removeTrailingForwardSlash(this.#manifest.base); this.#pipeline = new SSRRoutePipeline(this.#createEnvironment(streaming)); - this.#middlewareLoaded = false; this.#adapterLogger = new AstroIntegrationLogger( this.#logger.options, this.#manifest.adapterName @@ -137,20 +134,7 @@ export class App { return routeData; } - async #getOnRequest() { - if (this.#manifest.middlewareEntryPoint && !this.#middlewareLoaded) { - try { - const middleware = await import(this.#manifest.middlewareEntryPoint); - this.#pipeline.setMiddlewareFunction(middleware.onRequest as MiddlewareEndpointHandler); - } catch (e) { - this.#logger.warn('SSR', "Couldn't load the middleware entry point"); - } - } - this.#middlewareLoaded = true; - } - async render(request: Request, routeData?: RouteData, locals?: object): Promise { - await this.#getOnRequest(); // Handle requests with duplicate slashes gracefully by cloning with a cleaned-up request URL if (request.url !== collapseDuplicateSlashes(request.url)) { request = new Request(collapseDuplicateSlashes(request.url), request); @@ -178,6 +162,9 @@ export class App { ); let response; try { + if (mod.onRequest) { + this.#pipeline.setMiddlewareFunction(mod.onRequest as MiddlewareEndpointHandler); + } response = await this.#pipeline.renderRoute(renderContext, pageModule); } catch (err: any) { if (err instanceof EndpointNotFoundError) { @@ -295,6 +282,9 @@ export class App { status ); const page = (await mod.page()) as any; + if (mod.onRequest) { + this.#pipeline.setMiddlewareFunction(mod.onRequest as MiddlewareEndpointHandler); + } const response = await this.#pipeline.renderRoute(newRenderContext, page); return this.#mergeResponses(response, originalResponse); } catch {} @@ -319,7 +309,7 @@ export class App { const { statusText, headers } = oldResponse; - // If the the new response did not have a meaningful status, an override may have been provided + // If 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 diff --git a/packages/astro/src/core/app/types.ts b/packages/astro/src/core/app/types.ts index faeb9377e..8812d2c44 100644 --- a/packages/astro/src/core/app/types.ts +++ b/packages/astro/src/core/app/types.ts @@ -49,7 +49,6 @@ export type SSRManifest = { componentMetadata: SSRResult['componentMetadata']; pageModule?: SinglePageBuiltModule; pageMap?: Map; - middlewareEntryPoint: string | undefined; }; export type SerializedSSRManifest = Omit< diff --git a/packages/astro/src/core/build/buildPipeline.ts b/packages/astro/src/core/build/buildPipeline.ts index 1956eede7..ba1bfad0e 100644 --- a/packages/astro/src/core/build/buildPipeline.ts +++ b/packages/astro/src/core/build/buildPipeline.ts @@ -78,13 +78,6 @@ export class BuildPipeline extends Pipeline { return this.#manifest; } - async retrieveMiddlewareFunction() { - if (this.#internals.middlewareEntryPoint) { - const middleware = await import(this.#internals.middlewareEntryPoint.toString()); - this.setMiddlewareFunction(middleware.onRequest); - } - } - getLogger(): Logger { return this.getEnvironment().logger; } diff --git a/packages/astro/src/core/build/generate.ts b/packages/astro/src/core/build/generate.ts index 53a350342..df6b2c4b2 100644 --- a/packages/astro/src/core/build/generate.ts +++ b/packages/astro/src/core/build/generate.ts @@ -10,6 +10,7 @@ import type { ComponentInstance, GetStaticPathsItem, ImageTransform, + MiddlewareEndpointHandler, RouteData, RouteType, SSRError, @@ -135,7 +136,7 @@ export async function generatePages(opts: StaticBuildOptions, internals: BuildIn ); } const pipeline = new BuildPipeline(opts, internals, manifest); - await pipeline.retrieveMiddlewareFunction(); + const outFolder = ssr ? opts.settings.config.build.server : getOutDirWithinCwd(opts.settings.config.outDir); @@ -247,7 +248,10 @@ async function generatePage( .reduce(mergeInlineCss, []); const pageModulePromise = ssrEntry.page; - + const onRequest = ssrEntry.onRequest; + if (onRequest) { + pipeline.setMiddlewareFunction(onRequest as MiddlewareEndpointHandler); + } if (!pageModulePromise) { throw new Error( `Unable to find the module for ${pageData.component}. This is unexpected and likely a bug in Astro, please report.` @@ -604,8 +608,5 @@ export function createBuildManifest( ? new URL(settings.config.base, settings.config.site).toString() : settings.config.site, componentMetadata: internals.componentMetadata, - middlewareEntryPoint: internals.middlewareEntryPoint - ? internals.middlewareEntryPoint.toString() - : undefined, }; } diff --git a/packages/astro/src/core/build/plugins/plugin-manifest.ts b/packages/astro/src/core/build/plugins/plugin-manifest.ts index 7f6846a63..5a1a3ce76 100644 --- a/packages/astro/src/core/build/plugins/plugin-manifest.ts +++ b/packages/astro/src/core/build/plugins/plugin-manifest.ts @@ -237,9 +237,6 @@ function buildManifest( // Set this to an empty string so that the runtime knows not to try and load this. entryModules[BEFORE_HYDRATION_SCRIPT_ID] = ''; } - const isEdgeMiddleware = - // TODO: remove in Astro 4.0 - settings.config.build.excludeMiddleware || settings.adapter?.adapterFeatures?.edgeMiddleware; const ssrManifest: SerializedSSRManifest = { adapterName: opts.settings.adapter?.name ?? '', @@ -253,9 +250,6 @@ function buildManifest( clientDirectives: Array.from(settings.clientDirectives), entryModules, assets: staticFiles.map(prefixAssetPath), - middlewareEntryPoint: !isEdgeMiddleware - ? internals.middlewareEntryPoint?.toString() - : undefined, }; return ssrManifest; diff --git a/packages/astro/src/core/build/types.ts b/packages/astro/src/core/build/types.ts index c45249e7e..07f2404bc 100644 --- a/packages/astro/src/core/build/types.ts +++ b/packages/astro/src/core/build/types.ts @@ -4,6 +4,7 @@ import type { AstroSettings, ComponentInstance, ManifestData, + MiddlewareHandler, RouteData, RuntimeMode, SSRLoadedRenderer, @@ -51,6 +52,7 @@ export interface SinglePageBuiltModule { /** * The `onRequest` hook exported by the middleware */ + onRequest?: MiddlewareHandler; renderers: SSRLoadedRenderer[]; } diff --git a/packages/astro/src/core/redirects/component.ts b/packages/astro/src/core/redirects/component.ts index 93b1aa732..ae4dbb4fe 100644 --- a/packages/astro/src/core/redirects/component.ts +++ b/packages/astro/src/core/redirects/component.ts @@ -12,5 +12,6 @@ export const RedirectComponentInstance: ComponentInstance = { export const RedirectSinglePageBuiltModule: SinglePageBuiltModule = { page: () => Promise.resolve(RedirectComponentInstance), + onRequest: (_, next) => next(), renderers: [], }; diff --git a/packages/astro/src/vite-plugin-astro-server/plugin.ts b/packages/astro/src/vite-plugin-astro-server/plugin.ts index 26568715b..80da6d787 100644 --- a/packages/astro/src/vite-plugin-astro-server/plugin.ts +++ b/packages/astro/src/vite-plugin-astro-server/plugin.ts @@ -99,6 +99,5 @@ export function createDevelopmentManifest(settings: AstroSettings): SSRManifest ? new URL(settings.config.base, settings.config.site).toString() : settings.config.site, componentMetadata: new Map(), - middlewareEntryPoint: undefined, }; } diff --git a/packages/integrations/vercel/test/edge-middleware.test.js b/packages/integrations/vercel/test/edge-middleware.test.js index b0927ff7a..deaeb4160 100644 --- a/packages/integrations/vercel/test/edge-middleware.test.js +++ b/packages/integrations/vercel/test/edge-middleware.test.js @@ -19,7 +19,7 @@ describe('Vercel edge middleware', () => { }); // TODO: The path here seems to be inconsistent? - it.skip('with edge handle file, should successfully build the middleware', async () => { + it.skip('without edge handle file, should successfully build the middleware', async () => { const fixture = await loadFixture({ root: './fixtures/middleware-without-edge-file/', });