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 <nate@astro.build>
Co-authored-by: Nate Moore <natemoo-re@users.noreply.github.com>
This commit is contained in:
Emanuele Stoppa 2023-08-30 18:56:10 +01:00 committed by GitHub
parent d01c336c4d
commit d4a6ab7339
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
10 changed files with 22 additions and 38 deletions

View file

@ -0,0 +1,5 @@
---
'astro': patch
---
Correctly retrive middleware when using it in SSR enviroments.

View file

@ -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<Response> {
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

View file

@ -49,7 +49,6 @@ export type SSRManifest = {
componentMetadata: SSRResult['componentMetadata'];
pageModule?: SinglePageBuiltModule;
pageMap?: Map<ComponentPath, ImportComponentInstance>;
middlewareEntryPoint: string | undefined;
};
export type SerializedSSRManifest = Omit<

View file

@ -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;
}

View file

@ -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,
};
}

View file

@ -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;

View file

@ -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<unknown>;
renderers: SSRLoadedRenderer[];
}

View file

@ -12,5 +12,6 @@ export const RedirectComponentInstance: ComponentInstance = {
export const RedirectSinglePageBuiltModule: SinglePageBuiltModule = {
page: () => Promise.resolve(RedirectComponentInstance),
onRequest: (_, next) => next(),
renderers: [],
};

View file

@ -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,
};
}

View file

@ -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/',
});