From f02a2d6dfc507741d224c04e3b8db27ae0b7db80 Mon Sep 17 00:00:00 2001 From: Emanuele Stoppa Date: Thu, 25 Jul 2024 09:21:36 +0100 Subject: [PATCH 01/11] refactor: dynamically import middleware --- .changeset/gold-bees-enjoy.md | 5 ++++ packages/astro/src/container/index.ts | 18 ++++++++++----- packages/astro/src/container/pipeline.ts | 23 ++++++++++++++++++- packages/astro/src/core/app/common.ts | 5 ++-- packages/astro/src/core/app/index.ts | 7 +++--- packages/astro/src/core/app/pipeline.ts | 20 ++++++++++++++++ packages/astro/src/core/app/types.ts | 3 ++- packages/astro/src/core/base-pipeline.ts | 6 +++++ packages/astro/src/core/build/generate.ts | 13 +++++++++-- packages/astro/src/core/build/pipeline.ts | 23 ++++++++++++++++++- .../src/core/build/plugins/plugin-ssr.ts | 3 +-- .../src/core/middleware/noop-middleware.ts | 3 +++ packages/astro/src/core/render-context.ts | 10 +++++--- .../src/vite-plugin-astro-server/pipeline.ts | 16 +++++++++++++ .../src/vite-plugin-astro-server/plugin.ts | 8 +++++-- .../src/vite-plugin-astro-server/route.ts | 4 ++-- 16 files changed, 142 insertions(+), 25 deletions(-) create mode 100644 .changeset/gold-bees-enjoy.md create mode 100644 packages/astro/src/core/middleware/noop-middleware.ts diff --git a/.changeset/gold-bees-enjoy.md b/.changeset/gold-bees-enjoy.md new file mode 100644 index 000000000000..59f64bb6b768 --- /dev/null +++ b/.changeset/gold-bees-enjoy.md @@ -0,0 +1,5 @@ +--- +'astro': patch +--- + +TODO diff --git a/packages/astro/src/container/index.ts b/packages/astro/src/container/index.ts index fef82df2a382..42586d1a93c6 100644 --- a/packages/astro/src/container/index.ts +++ b/packages/astro/src/container/index.ts @@ -2,6 +2,7 @@ import './polyfill.js'; import { posix } from 'node:path'; import type { AstroConfig, + AstroMiddlewareInstance, AstroUserConfig, ComponentInstance, ContainerImportRendererFn, @@ -27,6 +28,7 @@ import { getParts, validateSegment } from '../core/routing/manifest/create.js'; import { getPattern } from '../core/routing/manifest/pattern.js'; import type { AstroComponentFactory } from '../runtime/server/index.js'; import { ContainerPipeline } from './pipeline.js'; +import { NOOP_MIDDLEWARE_FN } from '../core/middleware/noop-middleware.js'; /** * Options to be passed when rendering a route @@ -109,9 +111,11 @@ function createManifest( renderers?: SSRLoadedRenderer[], middleware?: MiddlewareHandler, ): SSRManifest { - const defaultMiddleware: MiddlewareHandler = (_, next) => { - return next(); - }; + function middlewareInstance(): AstroMiddlewareInstance { + return { + onRequest: middleware ?? NOOP_MIDDLEWARE_FN, + }; + } return { hrefRoot: import.meta.url, @@ -130,7 +134,7 @@ function createManifest( inlinedScripts: manifest?.inlinedScripts ?? new Map(), i18n: manifest?.i18n, checkOrigin: false, - middleware: manifest?.middleware ?? middleware ?? defaultMiddleware, + middleware: manifest?.middleware ?? middlewareInstance, experimentalEnvGetSecretEnabled: false, key: createKey(), }; @@ -476,11 +480,13 @@ export class experimental_AstroContainer { params: options.params, type: routeType, }); - const renderContext = RenderContext.create({ + const middlewareInstance = await this.#pipeline.middleware(); + const middleware = middlewareInstance.onRequest; + const renderContext = await RenderContext.create({ pipeline: this.#pipeline, routeData, status: 200, - middleware: this.#pipeline.middleware, + middleware, request, pathname: url.pathname, locals: options?.locals ?? {}, diff --git a/packages/astro/src/container/pipeline.ts b/packages/astro/src/container/pipeline.ts index 6a2af65ce384..8e4a04d7ceca 100644 --- a/packages/astro/src/container/pipeline.ts +++ b/packages/astro/src/container/pipeline.ts @@ -1,5 +1,6 @@ import type { ComponentInstance, + MiddlewareHandler, RewritePayload, RouteData, SSRElement, @@ -12,6 +13,9 @@ import { createStylesheetElementSet, } from '../core/render/ssr-element.js'; import { findRouteToRewrite } from '../core/routing/rewrite.js'; +import { NOOP_MIDDLEWARE_FN } from '../core/middleware/noop-middleware.js'; +import { sequence } from '../core/middleware/index.js'; +import { createOriginCheckMiddleware } from '../core/app/middlewares.js'; export class ContainerPipeline extends Pipeline { /** @@ -23,6 +27,23 @@ export class ContainerPipeline extends Pipeline { SinglePageBuiltModule >(); + resolvedMiddleware: MiddlewareHandler | undefined = undefined; + + async getMiddleware(): Promise { + if (this.resolvedMiddleware) { + return this.resolvedMiddleware; + } else { + const middlewareInstance = await this.middleware(); + const onRequest = middlewareInstance.onRequest ?? NOOP_MIDDLEWARE_FN; + if (this.manifest.checkOrigin) { + this.resolvedMiddleware = sequence(createOriginCheckMiddleware(), onRequest); + } else { + this.resolvedMiddleware = onRequest; + } + return this.resolvedMiddleware; + } + } + static create({ logger, manifest, @@ -88,7 +109,7 @@ export class ContainerPipeline extends Pipeline { return Promise.resolve(componentInstance); }, renderers: this.manifest.renderers, - onRequest: this.manifest.middleware, + onRequest: this.resolvedMiddleware, }); } diff --git a/packages/astro/src/core/app/common.ts b/packages/astro/src/core/app/common.ts index 7cfe1c5dd741..3d951267caeb 100644 --- a/packages/astro/src/core/app/common.ts +++ b/packages/astro/src/core/app/common.ts @@ -1,6 +1,7 @@ import { decodeKey } from '../encryption.js'; import { deserializeRouteData } from '../routing/manifest/serialization.js'; import type { RouteInfo, SSRManifest, SerializedSSRManifest } from './types.js'; +import { NOOP_MIDDLEWARE_FN } from '../middleware/noop-middleware.js'; export function deserializeManifest(serializedManifest: SerializedSSRManifest): SSRManifest { const routes: RouteInfo[] = []; @@ -23,8 +24,8 @@ export function deserializeManifest(serializedManifest: SerializedSSRManifest): return { // in case user middleware exists, this no-op middleware will be reassigned (see plugin-ssr.ts) - middleware(_, next) { - return next(); + middleware() { + return { onRequest: NOOP_MIDDLEWARE_FN }; }, ...serializedManifest, assets, diff --git a/packages/astro/src/core/app/index.ts b/packages/astro/src/core/app/index.ts index 8041dda3c6f7..6b606275aee9 100644 --- a/packages/astro/src/core/app/index.ts +++ b/packages/astro/src/core/app/index.ts @@ -24,6 +24,7 @@ import { createDefaultRoutes, injectDefaultRoutes } from '../routing/default.js' import { matchRoute } from '../routing/match.js'; import { createOriginCheckMiddleware } from './middlewares.js'; import { AppPipeline } from './pipeline.js'; +import { NOOP_MIDDLEWARE_FN } from '../middleware/noop-middleware.js'; export { deserializeManifest } from './common.js'; @@ -323,7 +324,7 @@ export class App { // Load route module. We also catch its error here if it fails on initialization const mod = await this.#pipeline.getModuleForRoute(routeData); - const renderContext = RenderContext.create({ + const renderContext = await RenderContext.create({ pipeline: this.#pipeline, locals, pathname, @@ -428,10 +429,10 @@ export class App { } const mod = await this.#pipeline.getModuleForRoute(errorRouteData); try { - const renderContext = RenderContext.create({ + const renderContext = await RenderContext.create({ locals, pipeline: this.#pipeline, - middleware: skipMiddleware ? (_, next) => next() : undefined, + middleware: skipMiddleware ? NOOP_MIDDLEWARE_FN : undefined, pathname: this.#getPathnameFromRequest(request), request, routeData: errorRouteData, diff --git a/packages/astro/src/core/app/pipeline.ts b/packages/astro/src/core/app/pipeline.ts index 4f753419a004..e237b0079d25 100644 --- a/packages/astro/src/core/app/pipeline.ts +++ b/packages/astro/src/core/app/pipeline.ts @@ -1,6 +1,7 @@ import type { ComponentInstance, ManifestData, + MiddlewareHandler, RewritePayload, RouteData, SSRElement, @@ -11,9 +12,28 @@ import type { SinglePageBuiltModule } from '../build/types.js'; import { RedirectSinglePageBuiltModule } from '../redirects/component.js'; import { createModuleScriptElement, createStylesheetElementSet } from '../render/ssr-element.js'; import { findRouteToRewrite } from '../routing/rewrite.js'; +import { sequence } from '../middleware/index.js'; +import { createOriginCheckMiddleware } from './middlewares.js'; +import { NOOP_MIDDLEWARE_FN } from '../middleware/noop-middleware.js'; export class AppPipeline extends Pipeline { #manifestData: ManifestData | undefined; + resolvedMiddleware: MiddlewareHandler | undefined = undefined; + + async getMiddleware(): Promise { + if (this.resolvedMiddleware) { + return this.resolvedMiddleware; + } else { + const middlewareInstance = await this.middleware(); + const onRequest = middlewareInstance.onRequest ?? NOOP_MIDDLEWARE_FN; + if (this.manifest.checkOrigin) { + this.resolvedMiddleware = sequence(createOriginCheckMiddleware(), onRequest); + } else { + this.resolvedMiddleware = onRequest; + } + return this.resolvedMiddleware; + } + } static create( manifestData: ManifestData, diff --git a/packages/astro/src/core/app/types.ts b/packages/astro/src/core/app/types.ts index 6a00ec0a7993..b86ea04788c3 100644 --- a/packages/astro/src/core/app/types.ts +++ b/packages/astro/src/core/app/types.ts @@ -7,6 +7,7 @@ import type { SSRLoadedRenderer, SSRResult, SerializedRouteData, + AstroMiddlewareInstance, } from '../../@types/astro.js'; import type { RoutingStrategies } from '../../i18n/utils.js'; import type { SinglePageBuiltModule } from '../build/types.js'; @@ -68,7 +69,7 @@ export type SSRManifest = { serverIslandNameMap?: Map; key: Promise; i18n: SSRManifestI18n | undefined; - middleware: MiddlewareHandler; + middleware: () => Promise | AstroMiddlewareInstance; checkOrigin: boolean; // TODO: remove experimental prefix experimentalEnvGetSecretEnabled: boolean; diff --git a/packages/astro/src/core/base-pipeline.ts b/packages/astro/src/core/base-pipeline.ts index 6742a8425597..0dfca7b48dd7 100644 --- a/packages/astro/src/core/base-pipeline.ts +++ b/packages/astro/src/core/base-pipeline.ts @@ -97,6 +97,12 @@ export abstract class Pipeline { * @param routeData */ abstract getComponentByRoute(routeData: RouteData): Promise; + + /** + * Resolves the middleware from the manifest, and returns the `onRequest` function. If `onRequest` isn't there, + * it returns a no-op function + */ + abstract getMiddleware(): Promise; } // eslint-disable-next-line @typescript-eslint/no-empty-object-type diff --git a/packages/astro/src/core/build/generate.ts b/packages/astro/src/core/build/generate.ts index b24fb17c416c..7cd2aa91e367 100644 --- a/packages/astro/src/core/build/generate.ts +++ b/packages/astro/src/core/build/generate.ts @@ -443,7 +443,12 @@ async function generatePath( logger, staticLike: true, }); - const renderContext = RenderContext.create({ pipeline, pathname, request, routeData: route }); + const renderContext = await RenderContext.create({ + pipeline, + pathname, + request, + routeData: route, + }); let body: string | Uint8Array; let response: Response; @@ -552,7 +557,11 @@ function createBuildManifest( componentMetadata: internals.componentMetadata, i18n: i18nManifest, buildFormat: settings.config.build.format, - middleware, + middleware() { + return { + onRequest: middleware, + }; + }, checkOrigin: settings.config.security?.checkOrigin ?? false, key, experimentalEnvGetSecretEnabled: false, diff --git a/packages/astro/src/core/build/pipeline.ts b/packages/astro/src/core/build/pipeline.ts index 760c353d1da1..f8a3e0701168 100644 --- a/packages/astro/src/core/build/pipeline.ts +++ b/packages/astro/src/core/build/pipeline.ts @@ -1,5 +1,6 @@ import type { ComponentInstance, + MiddlewareHandler, RewritePayload, RouteData, SSRLoadedRenderer, @@ -27,6 +28,9 @@ import { RESOLVED_SPLIT_MODULE_ID } from './plugins/plugin-ssr.js'; import { getPagesFromVirtualModulePageName, getVirtualModulePageName } from './plugins/util.js'; import type { PageBuildData, SinglePageBuiltModule, StaticBuildOptions } from './types.js'; import { i18nHasFallback } from './util.js'; +import { NOOP_MIDDLEWARE_FN } from '../middleware/noop-middleware.js'; +import { sequence } from '../middleware/index.js'; +import { createOriginCheckMiddleware } from '../app/middlewares.js'; /** * The build pipeline is responsible to gather the files emitted by the SSR build and generate the pages by executing these files. @@ -49,6 +53,19 @@ export class BuildPipeline extends Pipeline { : getOutDirWithinCwd(this.settings.config.outDir); } + resolvedMiddleware: MiddlewareHandler | undefined = undefined; + + async getMiddleware(): Promise { + if (this.resolvedMiddleware) { + return this.resolvedMiddleware; + } else { + const middlewareInstance = await this.middleware(); + const onRequest = middlewareInstance.onRequest ?? NOOP_MIDDLEWARE_FN; + this.resolvedMiddleware = onRequest; + return this.resolvedMiddleware; + } + } + private constructor( readonly internals: BuildInternals, readonly manifest: SSRManifest, @@ -138,7 +155,11 @@ export class BuildPipeline extends Pipeline { const renderers = await import(renderersEntryUrl.toString()); const middleware = await import(new URL('middleware.mjs', baseDirectory).toString()) - .then((mod) => mod.onRequest) + .then((mod) => { + return function () { + return { onRequest: mod.onRequest }; + }; + }) // middleware.mjs is not emitted if there is no user middleware // in which case the import fails with ERR_MODULE_NOT_FOUND, and we fall back to a no-op middleware .catch(() => manifest.middleware); diff --git a/packages/astro/src/core/build/plugins/plugin-ssr.ts b/packages/astro/src/core/build/plugins/plugin-ssr.ts index e9eda1dc90c2..883cdee38831 100644 --- a/packages/astro/src/core/build/plugins/plugin-ssr.ts +++ b/packages/astro/src/core/build/plugins/plugin-ssr.ts @@ -291,12 +291,11 @@ function generateSSRCode(settings: AstroSettings, adapter: AstroAdapter, middlew const contents = [ settings.config.experimental.serverIslands ? '' : `const serverIslandMap = new Map()`, - edgeMiddleware ? `const middleware = (_, next) => next()` : '', `const _manifest = Object.assign(defaultManifest, {`, ` ${pageMap},`, ` serverIslandMap,`, ` renderers,`, - ` middleware`, + ` middleware: ${edgeMiddleware ? 'undefined' : `() => import("${middlewareId}")`}`, `});`, `const _args = ${adapter.args ? JSON.stringify(adapter.args, null, 4) : 'undefined'};`, adapter.exports diff --git a/packages/astro/src/core/middleware/noop-middleware.ts b/packages/astro/src/core/middleware/noop-middleware.ts new file mode 100644 index 000000000000..bf5f10d89054 --- /dev/null +++ b/packages/astro/src/core/middleware/noop-middleware.ts @@ -0,0 +1,3 @@ +import type { MiddlewareHandler } from '../../@types/astro.js'; + +export const NOOP_MIDDLEWARE_FN: MiddlewareHandler = (_, next) => next(); diff --git a/packages/astro/src/core/render-context.ts b/packages/astro/src/core/render-context.ts index 9d813e19a381..62835d964c60 100644 --- a/packages/astro/src/core/render-context.ts +++ b/packages/astro/src/core/render-context.ts @@ -36,6 +36,7 @@ import { callMiddleware } from './middleware/callMiddleware.js'; import { sequence } from './middleware/index.js'; import { renderRedirect } from './redirects/render.js'; import { type Pipeline, Slots, getParams, getProps } from './render/index.js'; +import { NOOP_MIDDLEWARE_FN } from './middleware/noop-middleware.js'; export const apiContextRoutesSymbol = Symbol.for('context.routes'); @@ -67,7 +68,7 @@ export class RenderContext { */ counter = 0; - static create({ + static async create({ locals = {}, middleware, pathname, @@ -77,11 +78,14 @@ export class RenderContext { status = 200, props, }: Pick & - Partial>): RenderContext { + Partial< + Pick + >): Promise { + const pipelineMiddleware = await pipeline.getMiddleware(); return new RenderContext( pipeline, locals, - sequence(...pipeline.internalMiddleware, middleware ?? pipeline.middleware), + sequence(...pipeline.internalMiddleware, middleware ?? pipelineMiddleware), pathname, request, routeData, diff --git a/packages/astro/src/vite-plugin-astro-server/pipeline.ts b/packages/astro/src/vite-plugin-astro-server/pipeline.ts index 0bc069cbc8e7..d0986859def9 100644 --- a/packages/astro/src/vite-plugin-astro-server/pipeline.ts +++ b/packages/astro/src/vite-plugin-astro-server/pipeline.ts @@ -4,6 +4,7 @@ import type { ComponentInstance, DevToolbarMetadata, ManifestData, + MiddlewareHandler, RewritePayload, RouteData, SSRElement, @@ -27,6 +28,9 @@ import { getStylesForURL } from './css.js'; import { getComponentMetadata } from './metadata.js'; import { createResolve } from './resolve.js'; import { getScriptsForURL } from './scripts.js'; +import { NOOP_MIDDLEWARE_FN } from '../core/middleware/noop-middleware.js'; +import { sequence } from '../core/middleware/index.js'; +import { createOriginCheckMiddleware } from '../core/app/middlewares.js'; export class DevPipeline extends Pipeline { // renderers are loaded on every request, @@ -34,6 +38,18 @@ export class DevPipeline extends Pipeline { override renderers = new Array(); manifestData: ManifestData | undefined; + resolvedMiddleware: MiddlewareHandler | undefined = undefined; + + async getMiddleware(): Promise { + if (this.resolvedMiddleware) { + return this.resolvedMiddleware; + } else { + const middlewareInstance = await this.middleware(); + const onRequest = middlewareInstance.onRequest ?? NOOP_MIDDLEWARE_FN; + this.resolvedMiddleware = onRequest; + return this.resolvedMiddleware; + } + } componentInterner: WeakMap = new WeakMap< RouteData, diff --git a/packages/astro/src/vite-plugin-astro-server/plugin.ts b/packages/astro/src/vite-plugin-astro-server/plugin.ts index 19ceed811727..6633fa800083 100644 --- a/packages/astro/src/vite-plugin-astro-server/plugin.ts +++ b/packages/astro/src/vite-plugin-astro-server/plugin.ts @@ -152,8 +152,12 @@ export function createDevelopmentManifest(settings: AstroSettings): SSRManifest checkOrigin: settings.config.security?.checkOrigin ?? false, experimentalEnvGetSecretEnabled: false, key: createKey(), - middleware(_, next) { - return next(); + middleware() { + return { + onRequest(_, next) { + return next(); + }, + }; }, }; } diff --git a/packages/astro/src/vite-plugin-astro-server/route.ts b/packages/astro/src/vite-plugin-astro-server/route.ts index d6084dbdda22..30a6eda0190e 100644 --- a/packages/astro/src/vite-plugin-astro-server/route.ts +++ b/packages/astro/src/vite-plugin-astro-server/route.ts @@ -198,7 +198,7 @@ export async function handleRoute({ matchedRoute.route.prerender && matchedRoute.route.component === DEFAULT_404_COMPONENT; - renderContext = RenderContext.create({ + renderContext = await RenderContext.create({ locals, pipeline, pathname, @@ -235,7 +235,7 @@ export async function handleRoute({ req({ url: pathname, method: incomingRequest.method, - statusCode: isRewrite ? response.status : status ?? response.status, + statusCode: isRewrite ? response.status : (status ?? response.status), isRewrite, reqTime: timeEnd - timeStart, }), From e599752f275d21958c23f952d196504ca993e4dc Mon Sep 17 00:00:00 2001 From: Emanuele Stoppa Date: Thu, 29 Aug 2024 13:53:25 +0100 Subject: [PATCH 02/11] update tests --- packages/astro/test/units/render/head.test.js | 6 +++--- packages/astro/test/units/render/jsx.test.js | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/packages/astro/test/units/render/head.test.js b/packages/astro/test/units/render/head.test.js index 0eb4c77153d4..4c4a9eb77f9e 100644 --- a/packages/astro/test/units/render/head.test.js +++ b/packages/astro/test/units/render/head.test.js @@ -103,7 +103,7 @@ describe('core/render', () => { component: 'src/pages/index.astro', params: {}, }; - const renderContext = RenderContext.create({ pipeline, request, routeData }); + const renderContext = await RenderContext.create({ pipeline, request, routeData }); const response = await renderContext.render(PageModule); const html = await response.text(); @@ -184,7 +184,7 @@ describe('core/render', () => { component: 'src/pages/index.astro', params: {}, }; - const renderContext = RenderContext.create({ pipeline, request, routeData }); + const renderContext = await RenderContext.create({ pipeline, request, routeData }); const response = await renderContext.render(PageModule); const html = await response.text(); @@ -232,7 +232,7 @@ describe('core/render', () => { component: 'src/pages/index.astro', params: {}, }; - const renderContext = RenderContext.create({ pipeline, request, routeData }); + const renderContext = await RenderContext.create({ pipeline, request, routeData }); const response = await renderContext.render(PageModule); const html = await response.text(); diff --git a/packages/astro/test/units/render/jsx.test.js b/packages/astro/test/units/render/jsx.test.js index ba03a6f55d22..a42b819cea78 100644 --- a/packages/astro/test/units/render/jsx.test.js +++ b/packages/astro/test/units/render/jsx.test.js @@ -50,7 +50,7 @@ describe('core/render', () => { component: 'src/pages/index.mdx', params: {}, }; - const renderContext = RenderContext.create({ pipeline, request, routeData }); + const renderContext = await RenderContext.create({ pipeline, request, routeData }); const response = await renderContext.render(mod); assert.equal(response.status, 200); @@ -97,7 +97,7 @@ describe('core/render', () => { component: 'src/pages/index.mdx', params: {}, }; - const renderContext = RenderContext.create({ pipeline, request, routeData }); + const renderContext = await RenderContext.create({ pipeline, request, routeData }); const response = await renderContext.render(mod); assert.equal(response.status, 200); @@ -128,7 +128,7 @@ describe('core/render', () => { component: 'src/pages/index.mdx', params: {}, }; - const renderContext = RenderContext.create({ pipeline, request, routeData }); + const renderContext = await RenderContext.create({ pipeline, request, routeData }); const response = await renderContext.render(mod); try { From d1c570788e662c35226c73ea429071cc6db12ea3 Mon Sep 17 00:00:00 2001 From: Emanuele Stoppa Date: Thu, 29 Aug 2024 13:59:11 +0100 Subject: [PATCH 03/11] chore: fix code merge --- packages/astro/src/core/app/index.ts | 9 --------- 1 file changed, 9 deletions(-) diff --git a/packages/astro/src/core/app/index.ts b/packages/astro/src/core/app/index.ts index 6b606275aee9..97f11c82682c 100644 --- a/packages/astro/src/core/app/index.ts +++ b/packages/astro/src/core/app/index.ts @@ -11,7 +11,6 @@ import { getSetCookiesFromResponse } from '../cookies/index.js'; import { AstroError, AstroErrorData } from '../errors/index.js'; import { consoleLogDestination } from '../logger/console.js'; import { AstroIntegrationLogger, Logger } from '../logger/core.js'; -import { sequence } from '../middleware/index.js'; import { appendForwardSlash, joinPaths, @@ -22,7 +21,6 @@ import { RenderContext } from '../render-context.js'; import { createAssetLink } from '../render/ssr-element.js'; import { createDefaultRoutes, injectDefaultRoutes } from '../routing/default.js'; import { matchRoute } from '../routing/match.js'; -import { createOriginCheckMiddleware } from './middlewares.js'; import { AppPipeline } from './pipeline.js'; import { NOOP_MIDDLEWARE_FN } from '../middleware/noop-middleware.js'; @@ -112,13 +110,6 @@ export class App { * @private */ #createPipeline(manifestData: ManifestData, streaming = false) { - if (this.#manifest.checkOrigin) { - this.#manifest.middleware = sequence( - createOriginCheckMiddleware(), - this.#manifest.middleware, - ); - } - return AppPipeline.create(manifestData, { logger: this.#logger, manifest: this.#manifest, From 8e0b0293e2beaa29a749b229a64a76aa1e03f80c Mon Sep 17 00:00:00 2001 From: Emanuele Stoppa Date: Thu, 29 Aug 2024 14:07:14 +0100 Subject: [PATCH 04/11] address linting --- packages/astro/src/core/app/types.ts | 1 - packages/astro/src/core/build/pipeline.ts | 2 -- packages/astro/src/core/render-context.ts | 1 - packages/astro/src/vite-plugin-astro-server/pipeline.ts | 2 -- 4 files changed, 6 deletions(-) diff --git a/packages/astro/src/core/app/types.ts b/packages/astro/src/core/app/types.ts index b86ea04788c3..b268c010bc64 100644 --- a/packages/astro/src/core/app/types.ts +++ b/packages/astro/src/core/app/types.ts @@ -1,7 +1,6 @@ import type { ComponentInstance, Locales, - MiddlewareHandler, RouteData, SSRComponentMetadata, SSRLoadedRenderer, diff --git a/packages/astro/src/core/build/pipeline.ts b/packages/astro/src/core/build/pipeline.ts index f8a3e0701168..a0284a04b982 100644 --- a/packages/astro/src/core/build/pipeline.ts +++ b/packages/astro/src/core/build/pipeline.ts @@ -29,8 +29,6 @@ import { getPagesFromVirtualModulePageName, getVirtualModulePageName } from './p import type { PageBuildData, SinglePageBuiltModule, StaticBuildOptions } from './types.js'; import { i18nHasFallback } from './util.js'; import { NOOP_MIDDLEWARE_FN } from '../middleware/noop-middleware.js'; -import { sequence } from '../middleware/index.js'; -import { createOriginCheckMiddleware } from '../app/middlewares.js'; /** * The build pipeline is responsible to gather the files emitted by the SSR build and generate the pages by executing these files. diff --git a/packages/astro/src/core/render-context.ts b/packages/astro/src/core/render-context.ts index 62835d964c60..1658f6c23593 100644 --- a/packages/astro/src/core/render-context.ts +++ b/packages/astro/src/core/render-context.ts @@ -36,7 +36,6 @@ import { callMiddleware } from './middleware/callMiddleware.js'; import { sequence } from './middleware/index.js'; import { renderRedirect } from './redirects/render.js'; import { type Pipeline, Slots, getParams, getProps } from './render/index.js'; -import { NOOP_MIDDLEWARE_FN } from './middleware/noop-middleware.js'; export const apiContextRoutesSymbol = Symbol.for('context.routes'); diff --git a/packages/astro/src/vite-plugin-astro-server/pipeline.ts b/packages/astro/src/vite-plugin-astro-server/pipeline.ts index d0986859def9..216f39c1d0dc 100644 --- a/packages/astro/src/vite-plugin-astro-server/pipeline.ts +++ b/packages/astro/src/vite-plugin-astro-server/pipeline.ts @@ -29,8 +29,6 @@ import { getComponentMetadata } from './metadata.js'; import { createResolve } from './resolve.js'; import { getScriptsForURL } from './scripts.js'; import { NOOP_MIDDLEWARE_FN } from '../core/middleware/noop-middleware.js'; -import { sequence } from '../core/middleware/index.js'; -import { createOriginCheckMiddleware } from '../core/app/middlewares.js'; export class DevPipeline extends Pipeline { // renderers are loaded on every request, From 35b0cf4e2998026a9fc83ee27274c559a10c3e80 Mon Sep 17 00:00:00 2001 From: Emanuele Stoppa Date: Thu, 29 Aug 2024 14:16:16 +0100 Subject: [PATCH 05/11] remove changeset --- .changeset/gold-bees-enjoy.md | 5 ----- 1 file changed, 5 deletions(-) delete mode 100644 .changeset/gold-bees-enjoy.md diff --git a/.changeset/gold-bees-enjoy.md b/.changeset/gold-bees-enjoy.md deleted file mode 100644 index 59f64bb6b768..000000000000 --- a/.changeset/gold-bees-enjoy.md +++ /dev/null @@ -1,5 +0,0 @@ ---- -'astro': patch ---- - -TODO From 8f1cc9048ca3d7054566a0281451e9ba44d3d948 Mon Sep 17 00:00:00 2001 From: Emanuele Stoppa Date: Thu, 29 Aug 2024 14:26:52 +0100 Subject: [PATCH 06/11] feedback --- packages/astro/src/vite-plugin-astro-server/plugin.ts | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/packages/astro/src/vite-plugin-astro-server/plugin.ts b/packages/astro/src/vite-plugin-astro-server/plugin.ts index 6633fa800083..bb0dc98ed352 100644 --- a/packages/astro/src/vite-plugin-astro-server/plugin.ts +++ b/packages/astro/src/vite-plugin-astro-server/plugin.ts @@ -19,6 +19,7 @@ import { recordServerError } from './error.js'; import { DevPipeline } from './pipeline.js'; import { handleRequest } from './request.js'; import { setRouteError } from './server-state.js'; +import {NOOP_MIDDLEWARE_FN} from "../core/middleware/noop-middleware.js"; export interface AstroPluginOptions { settings: AstroSettings; @@ -154,9 +155,7 @@ export function createDevelopmentManifest(settings: AstroSettings): SSRManifest key: createKey(), middleware() { return { - onRequest(_, next) { - return next(); - }, + onRequest: NOOP_MIDDLEWARE_FN }; }, }; From 74bd11e817d437439766c2e565ec74c854e527b9 Mon Sep 17 00:00:00 2001 From: Emanuele Stoppa Date: Thu, 29 Aug 2024 14:27:05 +0100 Subject: [PATCH 07/11] feedback --- packages/astro/src/vite-plugin-astro-server/plugin.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/astro/src/vite-plugin-astro-server/plugin.ts b/packages/astro/src/vite-plugin-astro-server/plugin.ts index bb0dc98ed352..073bb8f6d68e 100644 --- a/packages/astro/src/vite-plugin-astro-server/plugin.ts +++ b/packages/astro/src/vite-plugin-astro-server/plugin.ts @@ -19,7 +19,7 @@ import { recordServerError } from './error.js'; import { DevPipeline } from './pipeline.js'; import { handleRequest } from './request.js'; import { setRouteError } from './server-state.js'; -import {NOOP_MIDDLEWARE_FN} from "../core/middleware/noop-middleware.js"; +import { NOOP_MIDDLEWARE_FN } from "../core/middleware/noop-middleware.js"; export interface AstroPluginOptions { settings: AstroSettings; From 0b76bb12aaa9dce1dfd84e112a3f94d146ee472c Mon Sep 17 00:00:00 2001 From: Emanuele Stoppa Date: Thu, 29 Aug 2024 15:17:45 +0100 Subject: [PATCH 08/11] fix test failing --- packages/astro/test/units/test-utils.js | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/packages/astro/test/units/test-utils.js b/packages/astro/test/units/test-utils.js index bab850b68530..ce84b7feaac7 100644 --- a/packages/astro/test/units/test-utils.js +++ b/packages/astro/test/units/test-utils.js @@ -13,6 +13,7 @@ import { nodeLogDestination } from '../../dist/core/logger/node.js'; import { Pipeline } from '../../dist/core/render/index.js'; import { RouteCache } from '../../dist/core/render/route-cache.js'; import { unixify } from './correct-path.js'; +import {NOOP_MIDDLEWARE_FN} from "../../dist/core/middleware/noop-middleware.js"; /** @type {import('../../src/core/logger/core').Logger} */ export const defaultLogger = new Logger({ @@ -207,6 +208,9 @@ export function createBasicPipeline(options = {}) { ); pipeline.headElements = () => ({ scripts: new Set(), styles: new Set(), links: new Set() }); pipeline.componentMetadata = () => new Map(); + pipeline.getMiddleware = () => { + return NOOP_MIDDLEWARE_FN; + } return pipeline; } From 565e988cc84c338c38a43ddb69142fc0a1e97772 Mon Sep 17 00:00:00 2001 From: Emanuele Stoppa Date: Fri, 30 Aug 2024 13:43:46 +0100 Subject: [PATCH 09/11] add suggestion --- packages/astro/src/container/pipeline.ts | 21 ------------------- packages/astro/src/core/app/pipeline.ts | 20 ------------------ packages/astro/src/core/base-pipeline.ts | 21 +++++++++++++++++-- packages/astro/src/core/build/pipeline.ts | 15 ------------- .../src/vite-plugin-astro-server/pipeline.ts | 14 ------------- 5 files changed, 19 insertions(+), 72 deletions(-) diff --git a/packages/astro/src/container/pipeline.ts b/packages/astro/src/container/pipeline.ts index 8e4a04d7ceca..0412da4df8a1 100644 --- a/packages/astro/src/container/pipeline.ts +++ b/packages/astro/src/container/pipeline.ts @@ -1,6 +1,5 @@ import type { ComponentInstance, - MiddlewareHandler, RewritePayload, RouteData, SSRElement, @@ -13,9 +12,6 @@ import { createStylesheetElementSet, } from '../core/render/ssr-element.js'; import { findRouteToRewrite } from '../core/routing/rewrite.js'; -import { NOOP_MIDDLEWARE_FN } from '../core/middleware/noop-middleware.js'; -import { sequence } from '../core/middleware/index.js'; -import { createOriginCheckMiddleware } from '../core/app/middlewares.js'; export class ContainerPipeline extends Pipeline { /** @@ -27,23 +23,6 @@ export class ContainerPipeline extends Pipeline { SinglePageBuiltModule >(); - resolvedMiddleware: MiddlewareHandler | undefined = undefined; - - async getMiddleware(): Promise { - if (this.resolvedMiddleware) { - return this.resolvedMiddleware; - } else { - const middlewareInstance = await this.middleware(); - const onRequest = middlewareInstance.onRequest ?? NOOP_MIDDLEWARE_FN; - if (this.manifest.checkOrigin) { - this.resolvedMiddleware = sequence(createOriginCheckMiddleware(), onRequest); - } else { - this.resolvedMiddleware = onRequest; - } - return this.resolvedMiddleware; - } - } - static create({ logger, manifest, diff --git a/packages/astro/src/core/app/pipeline.ts b/packages/astro/src/core/app/pipeline.ts index e237b0079d25..4f753419a004 100644 --- a/packages/astro/src/core/app/pipeline.ts +++ b/packages/astro/src/core/app/pipeline.ts @@ -1,7 +1,6 @@ import type { ComponentInstance, ManifestData, - MiddlewareHandler, RewritePayload, RouteData, SSRElement, @@ -12,28 +11,9 @@ import type { SinglePageBuiltModule } from '../build/types.js'; import { RedirectSinglePageBuiltModule } from '../redirects/component.js'; import { createModuleScriptElement, createStylesheetElementSet } from '../render/ssr-element.js'; import { findRouteToRewrite } from '../routing/rewrite.js'; -import { sequence } from '../middleware/index.js'; -import { createOriginCheckMiddleware } from './middlewares.js'; -import { NOOP_MIDDLEWARE_FN } from '../middleware/noop-middleware.js'; export class AppPipeline extends Pipeline { #manifestData: ManifestData | undefined; - resolvedMiddleware: MiddlewareHandler | undefined = undefined; - - async getMiddleware(): Promise { - if (this.resolvedMiddleware) { - return this.resolvedMiddleware; - } else { - const middlewareInstance = await this.middleware(); - const onRequest = middlewareInstance.onRequest ?? NOOP_MIDDLEWARE_FN; - if (this.manifest.checkOrigin) { - this.resolvedMiddleware = sequence(createOriginCheckMiddleware(), onRequest); - } else { - this.resolvedMiddleware = onRequest; - } - return this.resolvedMiddleware; - } - } static create( manifestData: ManifestData, diff --git a/packages/astro/src/core/base-pipeline.ts b/packages/astro/src/core/base-pipeline.ts index 0dfca7b48dd7..7f6a4096bceb 100644 --- a/packages/astro/src/core/base-pipeline.ts +++ b/packages/astro/src/core/base-pipeline.ts @@ -15,6 +15,9 @@ import { AstroErrorData } from './errors/index.js'; import type { Logger } from './logger/core.js'; import { RouteCache } from './render/route-cache.js'; import { createDefaultRoutes } from './routing/default.js'; +import {NOOP_MIDDLEWARE_FN} from "./middleware/noop-middleware.js"; +import {sequence} from "./middleware/index.js"; +import {createOriginCheckMiddleware} from "./app/middlewares.js"; /** * The `Pipeline` represents the static parts of rendering that do not change between requests. @@ -24,7 +27,8 @@ import { createDefaultRoutes } from './routing/default.js'; */ export abstract class Pipeline { readonly internalMiddleware: MiddlewareHandler[]; - + resolvedMiddleware: MiddlewareHandler | undefined = undefined; + constructor( readonly logger: Logger, readonly manifest: SSRManifest, @@ -102,7 +106,20 @@ export abstract class Pipeline { * Resolves the middleware from the manifest, and returns the `onRequest` function. If `onRequest` isn't there, * it returns a no-op function */ - abstract getMiddleware(): Promise; + async getMiddleware(): Promise { + if (this.resolvedMiddleware) { + return this.resolvedMiddleware; + } else { + const middlewareInstance = await this.middleware(); + const onRequest = middlewareInstance.onRequest ?? NOOP_MIDDLEWARE_FN; + if (this.manifest.checkOrigin) { + this.resolvedMiddleware = sequence(createOriginCheckMiddleware(), onRequest); + } else { + this.resolvedMiddleware = onRequest; + } + return this.resolvedMiddleware; + } + } } // eslint-disable-next-line @typescript-eslint/no-empty-object-type diff --git a/packages/astro/src/core/build/pipeline.ts b/packages/astro/src/core/build/pipeline.ts index a0284a04b982..a009e6130f57 100644 --- a/packages/astro/src/core/build/pipeline.ts +++ b/packages/astro/src/core/build/pipeline.ts @@ -1,6 +1,5 @@ import type { ComponentInstance, - MiddlewareHandler, RewritePayload, RouteData, SSRLoadedRenderer, @@ -28,7 +27,6 @@ import { RESOLVED_SPLIT_MODULE_ID } from './plugins/plugin-ssr.js'; import { getPagesFromVirtualModulePageName, getVirtualModulePageName } from './plugins/util.js'; import type { PageBuildData, SinglePageBuiltModule, StaticBuildOptions } from './types.js'; import { i18nHasFallback } from './util.js'; -import { NOOP_MIDDLEWARE_FN } from '../middleware/noop-middleware.js'; /** * The build pipeline is responsible to gather the files emitted by the SSR build and generate the pages by executing these files. @@ -51,19 +49,6 @@ export class BuildPipeline extends Pipeline { : getOutDirWithinCwd(this.settings.config.outDir); } - resolvedMiddleware: MiddlewareHandler | undefined = undefined; - - async getMiddleware(): Promise { - if (this.resolvedMiddleware) { - return this.resolvedMiddleware; - } else { - const middlewareInstance = await this.middleware(); - const onRequest = middlewareInstance.onRequest ?? NOOP_MIDDLEWARE_FN; - this.resolvedMiddleware = onRequest; - return this.resolvedMiddleware; - } - } - private constructor( readonly internals: BuildInternals, readonly manifest: SSRManifest, diff --git a/packages/astro/src/vite-plugin-astro-server/pipeline.ts b/packages/astro/src/vite-plugin-astro-server/pipeline.ts index 216f39c1d0dc..0bc069cbc8e7 100644 --- a/packages/astro/src/vite-plugin-astro-server/pipeline.ts +++ b/packages/astro/src/vite-plugin-astro-server/pipeline.ts @@ -4,7 +4,6 @@ import type { ComponentInstance, DevToolbarMetadata, ManifestData, - MiddlewareHandler, RewritePayload, RouteData, SSRElement, @@ -28,7 +27,6 @@ import { getStylesForURL } from './css.js'; import { getComponentMetadata } from './metadata.js'; import { createResolve } from './resolve.js'; import { getScriptsForURL } from './scripts.js'; -import { NOOP_MIDDLEWARE_FN } from '../core/middleware/noop-middleware.js'; export class DevPipeline extends Pipeline { // renderers are loaded on every request, @@ -36,18 +34,6 @@ export class DevPipeline extends Pipeline { override renderers = new Array(); manifestData: ManifestData | undefined; - resolvedMiddleware: MiddlewareHandler | undefined = undefined; - - async getMiddleware(): Promise { - if (this.resolvedMiddleware) { - return this.resolvedMiddleware; - } else { - const middlewareInstance = await this.middleware(); - const onRequest = middlewareInstance.onRequest ?? NOOP_MIDDLEWARE_FN; - this.resolvedMiddleware = onRequest; - return this.resolvedMiddleware; - } - } componentInterner: WeakMap = new WeakMap< RouteData, From e2eb6aa3dc8402482ff6c4451caab11be37d6294 Mon Sep 17 00:00:00 2001 From: Emanuele Stoppa Date: Fri, 30 Aug 2024 13:50:01 +0100 Subject: [PATCH 10/11] change order of resolution to cache the middleware before inserting a route --- packages/astro/src/container/index.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/astro/src/container/index.ts b/packages/astro/src/container/index.ts index 42586d1a93c6..76bb0bd028de 100644 --- a/packages/astro/src/container/index.ts +++ b/packages/astro/src/container/index.ts @@ -474,14 +474,13 @@ export class experimental_AstroContainer { routeType === 'endpoint' ? (component as unknown as ComponentInstance) : this.#wrapComponent(component, options.params); + const middleware = await this.#pipeline.getMiddleware(); const routeData = this.#insertRoute({ path: request.url, componentInstance, params: options.params, type: routeType, }); - const middlewareInstance = await this.#pipeline.middleware(); - const middleware = middlewareInstance.onRequest; const renderContext = await RenderContext.create({ pipeline: this.#pipeline, routeData, From 21ca7ab7691839fb3d42c2468fd83731a65e287c Mon Sep 17 00:00:00 2001 From: Emanuele Stoppa Date: Mon, 23 Sep 2024 16:16:09 +0100 Subject: [PATCH 11/11] address comment --- packages/astro/src/container/index.ts | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/packages/astro/src/container/index.ts b/packages/astro/src/container/index.ts index 76bb0bd028de..9aa4e2d2f2d9 100644 --- a/packages/astro/src/container/index.ts +++ b/packages/astro/src/container/index.ts @@ -22,13 +22,13 @@ import { validateConfig } from '../core/config/validate.js'; import { createKey } from '../core/encryption.js'; import { Logger } from '../core/logger/core.js'; import { nodeLogDestination } from '../core/logger/node.js'; +import { NOOP_MIDDLEWARE_FN } from '../core/middleware/noop-middleware.js'; import { removeLeadingForwardSlash } from '../core/path.js'; import { RenderContext } from '../core/render-context.js'; import { getParts, validateSegment } from '../core/routing/manifest/create.js'; import { getPattern } from '../core/routing/manifest/pattern.js'; import type { AstroComponentFactory } from '../runtime/server/index.js'; import { ContainerPipeline } from './pipeline.js'; -import { NOOP_MIDDLEWARE_FN } from '../core/middleware/noop-middleware.js'; /** * Options to be passed when rendering a route @@ -474,7 +474,6 @@ export class experimental_AstroContainer { routeType === 'endpoint' ? (component as unknown as ComponentInstance) : this.#wrapComponent(component, options.params); - const middleware = await this.#pipeline.getMiddleware(); const routeData = this.#insertRoute({ path: request.url, componentInstance, @@ -485,7 +484,6 @@ export class experimental_AstroContainer { pipeline: this.#pipeline, routeData, status: 200, - middleware, request, pathname: url.pathname, locals: options?.locals ?? {},