feat: remove experimental flag middleware (#7109)

ematipico · matthewp · web-flow · commit 101f03209814 · 2023-06-05T11:25:18.000-04:00
* fix: middleware for API endpoints

* feat: remove experimental flag middleware

* chore: rebase and update

* chore: check if physical file exists

* chore: restore change

* merge

* merge

* fix: remove options, not needed

* remove experimental from types

* chore: don't have the middleware inside the manifest

* Update how redirects work, slightly

---------

Co-authored-by: Matthew Phillips &lt;matthew@skypack.dev&gt;
diff --git a/.changeset/young-flies-allow.md b/.changeset/young-flies-allow.md
@@ -0,0 +1,5 @@
+---
+'astro': minor
+---
+
+Remove experimental flag for the middleware
diff --git a/examples/middleware/astro.config.mjs b/examples/middleware/astro.config.mjs
@@ -8,6 +8,6 @@ export default defineConfig({
 		mode: 'standalone',
 	}),
 	experimental: {
-		middleware: true,
+		middleware: true
 	},
 });
diff --git a/packages/astro/src/@types/astro.ts b/packages/astro/src/@types/astro.ts
@@ -108,7 +108,6 @@ export interface CLIFlags {
 	drafts?: boolean;
 	open?: boolean;
 	experimentalAssets?: boolean;
-	experimentalMiddleware?: boolean;
 	experimentalRedirects?: boolean;
 }
 
@@ -1189,27 +1188,6 @@ export interface AstroUserConfig {
 		 */
 		customClientDirectives?: boolean;
 
-		/**
-		 * @docs
-		 * @name experimental.middleware
-		 * @type {boolean}
-		 * @default `false`
-		 * @version 2.4.0
-		 * @description
-		 * Enable experimental support for Astro middleware.
-		 *
-		 * To enable this feature, set `experimental.middleware` to `true` in your Astro config:
-		 *
-		 * ```js
-		 * {
-		 * 	experimental: {
-		 *		middleware: true,
-		 * 	},
-		 * }
-		 * ```
-		 */
-		middleware?: boolean;
-
 		/**
 		 * @docs
 		 * @name experimental.hybridOutput
diff --git a/packages/astro/src/core/app/index.ts b/packages/astro/src/core/app/index.ts
@@ -15,7 +15,7 @@ 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 { RedirectComponentInstance } from '../redirects/index.js';
+import { RedirectSinglePageBuiltModule } from '../redirects/index.js';
 import {
 	createEnvironment,
 	createRenderContext,
@@ -29,6 +29,7 @@ import {
 	createStylesheetElementSet,
 } from '../render/ssr-element.js';
 import { matchRoute } from '../routing/match.js';
+import type { SinglePageBuiltModule } from '../build/types';
 export { deserializeManifest } from './common.js';
 
 const clientLocalsSymbol = Symbol.for('astro.locals');
@@ -171,9 +172,9 @@ export class App {
 		return getSetCookiesFromResponse(response);
 	}
 
-	async #getModuleForRoute(route: RouteData): Promise<ComponentInstance> {
+	async #getModuleForRoute(route: RouteData): Promise<SinglePageBuiltModule> {
 		if (route.type === 'redirect') {
-			return RedirectComponentInstance;
+			return RedirectSinglePageBuiltModule;
 		} else {
 			const importComponentInstance = this.#manifest.pageMap.get(route.component);
 			if (!importComponentInstance) {
@@ -182,14 +183,14 @@ export class App {
 				);
 			}
 			const built = await importComponentInstance();
-			return built.page();
+			return built;
 		}
 	}
 
 	async #renderPage(
 		request: Request,
 		routeData: RouteData,
-		mod: ComponentInstance,
+		page: SinglePageBuiltModule,
 		status = 200
 	): Promise<Response> {
 		const url = new URL(request.url);
@@ -214,6 +215,7 @@ export class App {
 		}
 
 		try {
+			const mod = (await page.page()) as any;
 			const renderContext = await createRenderContext({
 				request,
 				origin: url.origin,
@@ -224,7 +226,7 @@ export class App {
 				links,
 				route: routeData,
 				status,
-				mod: mod as any,
+				mod,
 				env: this.#env,
 			});
 
@@ -235,7 +237,7 @@ export class App {
 				site: this.#env.site,
 				adapterName: this.#env.adapterName,
 			});
-			const onRequest = this.#manifest.middleware?.onRequest;
+			const onRequest = page.middleware?.onRequest;
 			let response;
 			if (onRequest) {
 				response = await callMiddleware<Response>(
@@ -268,11 +270,12 @@ export class App {
 	async #callEndpoint(
 		request: Request,
 		routeData: RouteData,
-		mod: ComponentInstance,
+		page: SinglePageBuiltModule,
 		status = 200
 	): Promise<Response> {
 		const url = new URL(request.url);
 		const pathname = '/' + this.removeBase(url.pathname);
+		const mod = await page.page();
 		const handler = mod as unknown as EndpointHandler;
 
 		const ctx = await createRenderContext({
@@ -285,13 +288,7 @@ export class App {
 			mod: handler as any,
 		});
 
-		const result = await callEndpoint(
-			handler,
-			this.#env,
-			ctx,
-			this.#logging,
-			this.#manifest.middleware
-		);
+		const result = await callEndpoint(handler, this.#env, ctx, this.#logging, page.middleware);
 
 		if (result.type === 'response') {
 			if (result.response.headers.get('X-Astro-Response') === 'Not-Found') {
diff --git a/packages/astro/src/core/app/types.ts b/packages/astro/src/core/app/types.ts
@@ -1,6 +1,7 @@
 import type { MarkdownRenderingOptions } from '@astrojs/markdown-remark';
 import type {
 	AstroMiddlewareInstance,
+	ComponentInstance,
 	RouteData,
 	SerializedRouteData,
 	SSRComponentMetadata,
@@ -49,7 +50,6 @@ export interface SSRManifest {
 	entryModules: Record<string, string>;
 	assets: Set<string>;
 	componentMetadata: SSRResult['componentMetadata'];
-	middleware?: AstroMiddlewareInstance<unknown>;
 }
 
 export type SerializedSSRManifest = Omit<
diff --git a/packages/astro/src/core/build/generate.ts b/packages/astro/src/core/build/generate.ts
@@ -41,7 +41,7 @@ import { debug, info } from '../logger/core.js';
 import { callMiddleware } from '../middleware/callMiddleware.js';
 import {
 	getRedirectLocationOrThrow,
-	RedirectComponentInstance,
+	RedirectSinglePageBuiltModule,
 	routeIsRedirect,
 } from '../redirects/index.js';
 import { createEnvironment, createRenderContext, renderPage } from '../render/index.js';
@@ -69,10 +69,6 @@ import type {
 } from './types';
 import { getTimeStat } from './util.js';
 
-const StaticMiddlewareInstance: AstroMiddlewareInstance<unknown> = {
-	onRequest: (ctx, next) => next(),
-};
-
 function createEntryURL(filePath: string, outFolder: URL) {
 	return new URL('./' + filePath + `?time=${Date.now()}`, outFolder);
 }
@@ -94,11 +90,7 @@ async function getEntryForRedirectRoute(
 		}
 	}
 
-	return {
-		page: () => Promise.resolve(RedirectComponentInstance),
-		middleware: StaticMiddlewareInstance,
-		renderers: [],
-	};
+	return RedirectSinglePageBuiltModule;
 }
 
 function shouldSkipDraft(pageModule: ComponentInstance, settings: AstroSettings): boolean {
diff --git a/packages/astro/src/core/build/plugins/plugin-middleware.ts b/packages/astro/src/core/build/plugins/plugin-middleware.ts
@@ -1,6 +1,5 @@
 import type { Plugin as VitePlugin } from 'vite';
 import { MIDDLEWARE_PATH_SEGMENT_NAME } from '../../constants.js';
-import { addRollupInput } from '../add-rollup-input.js';
 import type { BuildInternals } from '../internal.js';
 import type { AstroBuildPlugin } from '../plugin';
 import type { StaticBuildOptions } from '../types';
@@ -13,14 +12,9 @@ export function vitePluginMiddleware(
 ): VitePlugin {
 	return {
 		name: '@astro/plugin-middleware',
-		options(options) {
-			if (opts.settings.config.experimental.middleware) {
-				return addRollupInput(options, [MIDDLEWARE_MODULE_ID]);
-			}
-		},
 
 		async resolveId(id) {
-			if (id === MIDDLEWARE_MODULE_ID && opts.settings.config.experimental.middleware) {
+			if (id === MIDDLEWARE_MODULE_ID) {
 				const middlewareId = await this.resolve(
 					`${opts.settings.config.srcDir.pathname}/${MIDDLEWARE_PATH_SEGMENT_NAME}`
 				);
diff --git a/packages/astro/src/core/build/plugins/plugin-pages.ts b/packages/astro/src/core/build/plugins/plugin-pages.ts
@@ -80,9 +80,10 @@ function vitePluginPages(opts: StaticBuildOptions, internals: BuildInternals): V
 						imports.push(`import { renderers } from "${RENDERERS_MODULE_ID}";`);
 						exports.push(`export { renderers };`);
 
-						if (opts.settings.config.experimental.middleware) {
-							imports.push(`import * as _middleware from "${MIDDLEWARE_MODULE_ID}";`);
-							exports.push(`export const middleware = _middleware;`);
+						const middlewareModule = await this.resolve(MIDDLEWARE_MODULE_ID);
+						if (middlewareModule) {
+							imports.push(`import * as middleware from "${middlewareModule.id}";`);
+							exports.push(`export { middleware };`);
 						}
 
 						return `${imports.join('\n')}${exports.join('\n')}`;
diff --git a/packages/astro/src/core/build/plugins/plugin-ssr.ts b/packages/astro/src/core/build/plugins/plugin-ssr.ts
@@ -48,11 +48,6 @@ function vitePluginSSR(
 				const imports: string[] = [];
 				const contents: string[] = [];
 				const exports: string[] = [];
-				let middleware;
-				if (config.experimental?.middleware === true) {
-					imports.push(`import * as _middleware from "${MIDDLEWARE_MODULE_ID}"`);
-					middleware = 'middleware: _middleware';
-				}
 				let i = 0;
 				const pageMap: string[] = [];
 
@@ -84,7 +79,6 @@ import { _privateSetManifestDontUseThis } from 'astro:ssr-manifest';
 const _manifest = Object.assign(_deserializeManifest('${manifestReplace}'), {
 	pageMap,
 	renderers,
-	${middleware}
 });
 _privateSetManifestDontUseThis(_manifest);
 const _args = ${adapter.args ? JSON.stringify(adapter.args) : 'undefined'};
diff --git a/packages/astro/src/core/build/static-build.ts b/packages/astro/src/core/build/static-build.ts
@@ -179,9 +179,9 @@ async function ssrBuild(
 							return makeAstroPageEntryPointFileName(chunkInfo.facadeModuleId);
 						} else if (
 							// checks if the path of the module we have middleware, e.g. middleware.js / middleware/index.js
-							chunkInfo.facadeModuleId?.includes('middleware') &&
+							chunkInfo.moduleIds.find((m) => m.includes('middleware')) !== undefined &&
 							// checks if the file actually export the `onRequest` function
-							chunkInfo.exports.includes('onRequest')
+							chunkInfo.exports.includes('_middleware')
 						) {
 							return 'middleware.mjs';
 						} else if (chunkInfo.facadeModuleId === SSR_VIRTUAL_MODULE_ID) {
diff --git a/packages/astro/src/core/config/config.ts b/packages/astro/src/core/config/config.ts
@@ -104,8 +104,6 @@ export function resolveFlags(flags: Partial<Flags>): CLIFlags {
 		drafts: typeof flags.drafts === 'boolean' ? flags.drafts : undefined,
 		experimentalAssets:
 			typeof flags.experimentalAssets === 'boolean' ? flags.experimentalAssets : undefined,
-		experimentalMiddleware:
-			typeof flags.experimentalMiddleware === 'boolean' ? flags.experimentalMiddleware : undefined,
 		experimentalRedirects:
 			typeof flags.experimentalRedirects === 'boolean' ? flags.experimentalRedirects : undefined,
 	};
@@ -141,9 +139,6 @@ function mergeCLIFlags(astroConfig: AstroUserConfig, flags: CLIFlags) {
 		// TODO: Come back here and refactor to remove this expected error.
 		astroConfig.server.open = flags.open;
 	}
-	if (typeof flags.experimentalMiddleware === 'boolean') {
-		astroConfig.experimental.middleware = true;
-	}
 	return astroConfig;
 }
 
diff --git a/packages/astro/src/core/config/schema.ts b/packages/astro/src/core/config/schema.ts
@@ -212,7 +212,6 @@ export const AstroConfigSchema = z.object({
 				.boolean()
 				.optional()
 				.default(ASTRO_CONFIG_DEFAULTS.experimental.customClientDirecives),
-			middleware: z.oboolean().optional().default(ASTRO_CONFIG_DEFAULTS.experimental.middleware),
 			hybridOutput: z.boolean().optional().default(ASTRO_CONFIG_DEFAULTS.experimental.hybridOutput),
 			redirects: z.boolean().optional().default(ASTRO_CONFIG_DEFAULTS.experimental.redirects),
 		})
diff --git a/packages/astro/src/core/redirects/component.ts b/packages/astro/src/core/redirects/component.ts
@@ -1,4 +1,5 @@
-import type { ComponentInstance } from '../../@types/astro';
+import type { AstroMiddlewareInstance, ComponentInstance } from '../../@types/astro';
+import type { SinglePageBuiltModule } from '../build/types';
 
 // A stub of a component instance for a given route
 export const RedirectComponentInstance: ComponentInstance = {
@@ -8,3 +9,13 @@ export const RedirectComponentInstance: ComponentInstance = {
 		});
 	},
 };
+
+const StaticMiddlewareInstance: AstroMiddlewareInstance<unknown> = {
+	onRequest: (ctx, next) => next(),
+};
+
+export const RedirectSinglePageBuiltModule: SinglePageBuiltModule = {
+	page: () => Promise.resolve(RedirectComponentInstance),
+	middleware: StaticMiddlewareInstance,
+	renderers: [],
+}
diff --git a/packages/astro/src/core/redirects/index.ts b/packages/astro/src/core/redirects/index.ts
@@ -1,3 +1,3 @@
-export { RedirectComponentInstance } from './component.js';
+export { RedirectComponentInstance, RedirectSinglePageBuiltModule } from './component.js';
 export { redirectRouteGenerate, redirectRouteStatus, routeIsRedirect } from './helpers.js';
 export { getRedirectLocationOrThrow } from './validate.js';
diff --git a/packages/astro/src/vite-plugin-astro-server/route.ts b/packages/astro/src/vite-plugin-astro-server/route.ts
@@ -180,11 +180,9 @@ export async function handleRoute(
 		request,
 		route,
 	};
-	if (env.settings.config.experimental.middleware) {
-		const middleware = await loadMiddleware(env.loader, env.settings.config.srcDir);
-		if (middleware) {
-			options.middleware = middleware;
-		}
+	const middleware = await loadMiddleware(env.loader, env.settings.config.srcDir);
+	if (middleware) {
+		options.middleware = middleware;
 	}
 	// Route successfully matched! Render it.
 	if (route.type === 'endpoint') {
diff --git a/packages/astro/test/middleware.test.js b/packages/astro/test/middleware.test.js
@@ -109,9 +109,7 @@ describe('Middleware API in PROD mode, SSR', () => {
 		fixture = await loadFixture({
 			root: './fixtures/middleware-dev/',
 			output: 'server',
-			adapter: testAdapter({
-				// exports: ['manifest', 'createApp', 'middleware'],
-			}),
+			adapter: testAdapter({}),
 		});
 		await fixture.build();
 	});

-Original file line number
+Diff line change
@@ @@ -0,0 +1,5 @@ @@
 +---
 +'astro': minor
 +---
++
 +Remove experimental flag for the middleware