Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/large-bugs-matter.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'astro': patch
---

Correctly retrive middleware when using it in SSR enviroments.
24 changes: 7 additions & 17 deletions packages/astro/src/core/app/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -56,8 +56,6 @@ export class App {
});
#baseWithoutTrailingSlash: string;
#pipeline: SSRRoutePipeline;
#onRequest: MiddlewareEndpointHandler | undefined;
#middlewareLoaded: boolean;
#adapterLogger: AstroIntegrationLogger;

constructor(manifest: SSRManifest, streaming = true) {
Expand All @@ -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
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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 {}
Expand All @@ -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
Expand Down
1 change: 0 additions & 1 deletion packages/astro/src/core/app/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,6 @@ export type SSRManifest = {
componentMetadata: SSRResult['componentMetadata'];
pageModule?: SinglePageBuiltModule;
pageMap?: Map<ComponentPath, ImportComponentInstance>;
middlewareEntryPoint: string | undefined;
};

export type SerializedSSRManifest = Omit<
Expand Down
7 changes: 0 additions & 7 deletions packages/astro/src/core/build/buildPipeline.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
11 changes: 6 additions & 5 deletions packages/astro/src/core/build/generate.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import type {
ComponentInstance,
GetStaticPathsItem,
ImageTransform,
MiddlewareEndpointHandler,
RouteData,
RouteType,
SSRError,
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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.`
Expand Down Expand Up @@ -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,
};
}
6 changes: 0 additions & 6 deletions packages/astro/src/core/build/plugins/plugin-manifest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 ?? '',
Expand All @@ -253,9 +250,6 @@ function buildManifest(
clientDirectives: Array.from(settings.clientDirectives),
entryModules,
assets: staticFiles.map(prefixAssetPath),
middlewareEntryPoint: !isEdgeMiddleware
? internals.middlewareEntryPoint?.toString()
: undefined,
};

return ssrManifest;
Expand Down
2 changes: 2 additions & 0 deletions packages/astro/src/core/build/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import type {
AstroSettings,
ComponentInstance,
ManifestData,
MiddlewareHandler,
RouteData,
RuntimeMode,
SSRLoadedRenderer,
Expand Down Expand Up @@ -51,6 +52,7 @@ export interface SinglePageBuiltModule {
/**
* The `onRequest` hook exported by the middleware
*/
onRequest?: MiddlewareHandler<unknown>;
renderers: SSRLoadedRenderer[];
}

Expand Down
1 change: 1 addition & 0 deletions packages/astro/src/core/redirects/component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,5 +12,6 @@ export const RedirectComponentInstance: ComponentInstance = {

export const RedirectSinglePageBuiltModule: SinglePageBuiltModule = {
page: () => Promise.resolve(RedirectComponentInstance),
onRequest: (_, next) => next(),
renderers: [],
};
1 change: 0 additions & 1 deletion packages/astro/src/vite-plugin-astro-server/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
};
}
2 changes: 1 addition & 1 deletion packages/integrations/vercel/test/edge-middleware.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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/',
});
Expand Down