Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: dynamically import middleware #11550

Merged
merged 11 commits into from
Sep 23, 2024
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
15 changes: 9 additions & 6 deletions packages/astro/src/container/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import './polyfill.js';
import { posix } from 'node:path';
import type {
AstroConfig,
AstroMiddlewareInstance,
AstroUserConfig,
ComponentInstance,
ContainerImportRendererFn,
Expand All @@ -21,6 +22,7 @@ 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';
Expand Down Expand Up @@ -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,
Expand All @@ -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(),
};
Expand Down Expand Up @@ -476,11 +480,10 @@ export class experimental_AstroContainer {
params: options.params,
type: routeType,
});
const renderContext = RenderContext.create({
const renderContext = await RenderContext.create({
pipeline: this.#pipeline,
routeData,
status: 200,
middleware: this.#pipeline.middleware,
request,
pathname: url.pathname,
locals: options?.locals ?? {},
Expand Down
2 changes: 1 addition & 1 deletion packages/astro/src/container/pipeline.ts
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ export class ContainerPipeline extends Pipeline {
return Promise.resolve(componentInstance);
},
renderers: this.manifest.renderers,
onRequest: this.manifest.middleware,
onRequest: this.resolvedMiddleware,
ematipico marked this conversation as resolved.
Show resolved Hide resolved
});
}

Expand Down
5 changes: 3 additions & 2 deletions packages/astro/src/core/app/common.ts
Original file line number Diff line number Diff line change
@@ -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[] = [];
Expand All @@ -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,
Expand Down
16 changes: 4 additions & 12 deletions packages/astro/src/core/app/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -22,8 +21,8 @@ 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';

export { deserializeManifest } from './common.js';

Expand Down Expand Up @@ -111,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,
Expand Down Expand Up @@ -323,7 +315,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,
Expand Down Expand Up @@ -428,10 +420,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,
Expand Down
4 changes: 2 additions & 2 deletions packages/astro/src/core/app/types.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
import type {
ComponentInstance,
Locales,
MiddlewareHandler,
RouteData,
SSRComponentMetadata,
SSRLoadedRenderer,
SSRResult,
SerializedRouteData,
AstroMiddlewareInstance,
} from '../../@types/astro.js';
import type { RoutingStrategies } from '../../i18n/utils.js';
import type { SinglePageBuiltModule } from '../build/types.js';
Expand Down Expand Up @@ -68,7 +68,7 @@ export type SSRManifest = {
serverIslandNameMap?: Map<string, string>;
key: Promise<CryptoKey>;
i18n: SSRManifestI18n | undefined;
middleware: MiddlewareHandler;
middleware: () => Promise<AstroMiddlewareInstance> | AstroMiddlewareInstance;
checkOrigin: boolean;
// TODO: remove experimental prefix
experimentalEnvGetSecretEnabled: boolean;
Expand Down
25 changes: 24 additions & 1 deletion packages/astro/src/core/base-pipeline.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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,
Expand Down Expand Up @@ -97,6 +101,25 @@ export abstract class Pipeline {
* @param routeData
*/
abstract getComponentByRoute(routeData: RouteData): Promise<ComponentInstance>;

/**
* Resolves the middleware from the manifest, and returns the `onRequest` function. If `onRequest` isn't there,
* it returns a no-op function
*/
async getMiddleware(): Promise<MiddlewareHandler> {
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
Expand Down
13 changes: 11 additions & 2 deletions packages/astro/src/core/build/generate.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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,
Expand Down
6 changes: 5 additions & 1 deletion packages/astro/src/core/build/pipeline.ts
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,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);
Expand Down
3 changes: 1 addition & 2 deletions packages/astro/src/core/build/plugins/plugin-ssr.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
3 changes: 3 additions & 0 deletions packages/astro/src/core/middleware/noop-middleware.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
import type { MiddlewareHandler } from '../../@types/astro.js';

export const NOOP_MIDDLEWARE_FN: MiddlewareHandler = (_, next) => next();
9 changes: 6 additions & 3 deletions packages/astro/src/core/render-context.ts
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ export class RenderContext {
*/
counter = 0;

static create({
static async create({
locals = {},
middleware,
pathname,
Expand All @@ -77,11 +77,14 @@ export class RenderContext {
status = 200,
props,
}: Pick<RenderContext, 'pathname' | 'pipeline' | 'request' | 'routeData'> &
Partial<Pick<RenderContext, 'locals' | 'middleware' | 'status' | 'props'>>): RenderContext {
Partial<
Pick<RenderContext, 'locals' | 'middleware' | 'status' | 'props'>
>): Promise<RenderContext> {
const pipelineMiddleware = await pipeline.getMiddleware();
return new RenderContext(
pipeline,
locals,
sequence(...pipeline.internalMiddleware, middleware ?? pipeline.middleware),
sequence(...pipeline.internalMiddleware, middleware ?? pipelineMiddleware),
pathname,
request,
routeData,
Expand Down
7 changes: 5 additions & 2 deletions packages/astro/src/vite-plugin-astro-server/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -152,8 +153,10 @@ export function createDevelopmentManifest(settings: AstroSettings): SSRManifest
checkOrigin: settings.config.security?.checkOrigin ?? false,
experimentalEnvGetSecretEnabled: false,
key: createKey(),
middleware(_, next) {
return next();
middleware() {
return {
onRequest: NOOP_MIDDLEWARE_FN
};
},
};
}
4 changes: 2 additions & 2 deletions packages/astro/src/vite-plugin-astro-server/route.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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,
}),
Expand Down
6 changes: 3 additions & 3 deletions packages/astro/test/units/render/head.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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();
Expand Down
6 changes: 3 additions & 3 deletions packages/astro/test/units/render/jsx.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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 {
Expand Down
4 changes: 4 additions & 0 deletions packages/astro/test/units/test-utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -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({
Expand Down Expand Up @@ -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;
}

Expand Down
Loading