From 101f032098148b3daaac8d46ff1e535b79232e43 Mon Sep 17 00:00:00 2001 From: Emanuele Stoppa Date: Mon, 5 Jun 2023 17:25:18 +0200 Subject: [PATCH] feat: remove experimental flag middleware (#7109) * 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 --- .changeset/young-flies-allow.md | 5 ++++ examples/middleware/astro.config.mjs | 2 +- packages/astro/src/@types/astro.ts | 22 --------------- packages/astro/src/core/app/index.ts | 27 +++++++++---------- packages/astro/src/core/app/types.ts | 2 +- packages/astro/src/core/build/generate.ts | 12 ++------- .../core/build/plugins/plugin-middleware.ts | 8 +----- .../src/core/build/plugins/plugin-pages.ts | 7 ++--- .../src/core/build/plugins/plugin-ssr.ts | 6 ----- packages/astro/src/core/build/static-build.ts | 4 +-- packages/astro/src/core/config/config.ts | 5 ---- packages/astro/src/core/config/schema.ts | 1 - .../astro/src/core/redirects/component.ts | 13 ++++++++- packages/astro/src/core/redirects/index.ts | 2 +- .../src/vite-plugin-astro-server/route.ts | 8 +++--- packages/astro/test/middleware.test.js | 4 +-- 16 files changed, 45 insertions(+), 83 deletions(-) create mode 100644 .changeset/young-flies-allow.md diff --git a/.changeset/young-flies-allow.md b/.changeset/young-flies-allow.md new file mode 100644 index 000000000..bac3dd638 --- /dev/null +++ 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 index 1d4662423..e93513956 100644 --- 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 index 29a77f2eb..b8122f6bd 100644 --- 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 index e047c0360..a3317c393 100644 --- 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 { + async #getModuleForRoute(route: RouteData): Promise { 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 { 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( @@ -268,11 +270,12 @@ export class App { async #callEndpoint( request: Request, routeData: RouteData, - mod: ComponentInstance, + page: SinglePageBuiltModule, status = 200 ): Promise { 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 index 0682acb6a..57d123fdb 100644 --- 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; assets: Set; componentMetadata: SSRResult['componentMetadata']; - middleware?: AstroMiddlewareInstance; } export type SerializedSSRManifest = Omit< diff --git a/packages/astro/src/core/build/generate.ts b/packages/astro/src/core/build/generate.ts index 42f930ed3..3009949e8 100644 --- 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 = { - 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 index dd9872da3..6bd63b44f 100644 --- 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 index d3ecfe7b8..f767a9c5b 100644 --- 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 index 7099fe3fe..79da987ee 100644 --- 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 index cceaa0361..68387a9d4 100644 --- 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 index 63adadc73..b2fc7e69e 100644 --- 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): 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 index 493d57a16..d40f2c28b 100644 --- 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 index 3e279390f..dec8e04c9 100644 --- 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 = { + 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 index dfae00b94..4f705afdf 100644 --- 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 index 5388ec960..dc0836fbd 100644 --- 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 index 432e6b983..e2c57bafb 100644 --- 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(); });