Skip to content

Commit

Permalink
refactor: singular middleware (#9776)
Browse files Browse the repository at this point in the history
* per-route middleware -> singular middleware

* add changeset

* emit middleware.mjs only when user middleware exists

* Apply suggestions from code review
  • Loading branch information
lilnasy authored Jan 25, 2024
1 parent b2197a2 commit dc75180
Show file tree
Hide file tree
Showing 11 changed files with 100 additions and 129 deletions.
5 changes: 5 additions & 0 deletions .changeset/twenty-papayas-agree.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"astro": patch
---

Simplifies internals that handle middleware.
2 changes: 2 additions & 0 deletions packages/astro/src/core/app/common.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ export function deserializeManifest(serializedManifest: SerializedSSRManifest):
const clientDirectives = new Map(serializedManifest.clientDirectives);

return {
// in case user middleware exists, this no-op middleware will be reassigned (see plugin-ssr.ts)
middleware(_, next) { return next() },
...serializedManifest,
assets,
componentMetadata,
Expand Down
18 changes: 6 additions & 12 deletions packages/astro/src/core/app/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ export interface RenderErrorOptions {
response?: Response;
status: 404 | 500;
/**
* Whether to skip onRequest() while rendering the error page. Defaults to false.
* Whether to skip middleware while rendering the error page. Defaults to false.
*/
skipMiddleware?: boolean;
}
Expand Down Expand Up @@ -260,16 +260,10 @@ export class App {
this.#manifest.buildFormat
);
if (i18nMiddleware) {
if (mod.onRequest) {
this.#pipeline.setMiddlewareFunction(sequence(i18nMiddleware, mod.onRequest));
} else {
this.#pipeline.setMiddlewareFunction(i18nMiddleware);
}
this.#pipeline.setMiddlewareFunction(sequence(i18nMiddleware, this.#manifest.middleware));
this.#pipeline.onBeforeRenderRoute(i18nPipelineHook);
} else {
if (mod.onRequest) {
this.#pipeline.setMiddlewareFunction(mod.onRequest);
}
this.#pipeline.setMiddlewareFunction(this.#manifest.middleware);
}
response = await this.#pipeline.renderRoute(renderContext, pageModule);
} catch (err: any) {
Expand Down Expand Up @@ -429,8 +423,8 @@ export class App {
status
);
const page = (await mod.page()) as any;
if (skipMiddleware === false && mod.onRequest) {
this.#pipeline.setMiddlewareFunction(mod.onRequest);
if (skipMiddleware === false) {
this.#pipeline.setMiddlewareFunction(this.#manifest.middleware);
}
if (skipMiddleware) {
// make sure middleware set by other requests is cleared out
Expand All @@ -440,7 +434,7 @@ export class App {
return this.#mergeResponses(response, originalResponse);
} catch {
// Middleware may be the cause of the error, so we try rendering 404/500.astro without it.
if (skipMiddleware === false && mod.onRequest) {
if (skipMiddleware === false) {
return this.#renderError(request, {
status,
response: originalResponse,
Expand Down
4 changes: 3 additions & 1 deletion packages/astro/src/core/app/types.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import type {
Locales,
MiddlewareHandler,
RouteData,
SerializedRouteData,
SSRComponentMetadata,
Expand Down Expand Up @@ -54,6 +55,7 @@ export type SSRManifest = {
pageModule?: SinglePageBuiltModule;
pageMap?: Map<ComponentPath, ImportComponentInstance>;
i18n: SSRManifestI18n | undefined;
middleware: MiddlewareHandler;
};

export type SSRManifestI18n = {
Expand All @@ -65,7 +67,7 @@ export type SSRManifestI18n = {

export type SerializedSSRManifest = Omit<
SSRManifest,
'routes' | 'assets' | 'componentMetadata' | 'clientDirectives'
'middleware' | 'routes' | 'assets' | 'componentMetadata' | 'clientDirectives'
> & {
routes: SerializedRouteInfo[];
assets: string[];
Expand Down
35 changes: 21 additions & 14 deletions packages/astro/src/core/build/generate.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import type {
AstroSettings,
ComponentInstance,
GetStaticPathsItem,
MiddlewareHandler,
RouteData,
RouteType,
SSRError,
Expand Down Expand Up @@ -145,10 +146,17 @@ export async function generatePages(opts: StaticBuildOptions, internals: BuildIn
const baseDirectory = getOutputDirectory(opts.settings.config);
const renderersEntryUrl = new URL('renderers.mjs', baseDirectory);
const renderers = await import(renderersEntryUrl.toString());
let middleware: MiddlewareHandler = (_, next) => next();
try {
// 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
middleware = await import(new URL('middleware.mjs', baseDirectory).toString()).then(mod => mod.onRequest);
} catch {}
manifest = createBuildManifest(
opts.settings,
internals,
renderers.renderers as SSRLoadedRenderer[]
renderers.renderers as SSRLoadedRenderer[],
middleware
);
}
const pipeline = new BuildPipeline(opts, internals, manifest);
Expand Down Expand Up @@ -249,8 +257,9 @@ async function generatePage(
// prepare information we need
const logger = pipeline.getLogger();
const config = pipeline.getConfig();
const manifest = pipeline.getManifest();
const pageModulePromise = ssrEntry.page;
const onRequest = ssrEntry.onRequest;
const onRequest = manifest.middleware;
const pageInfo = getPageDataByComponent(pipeline.getInternals(), pageData.route.component);

// Calculate information of the page, like scripts, links and styles
Expand All @@ -263,19 +272,15 @@ async function generatePage(
const scripts = pageInfo?.hoistedScript ?? null;
// prepare the middleware
const i18nMiddleware = createI18nMiddleware(
pipeline.getManifest().i18n,
pipeline.getManifest().base,
pipeline.getManifest().trailingSlash,
pipeline.getManifest().buildFormat
manifest.i18n,
manifest.base,
manifest.trailingSlash,
manifest.buildFormat
);
if (config.i18n && i18nMiddleware) {
if (onRequest) {
pipeline.setMiddlewareFunction(sequence(i18nMiddleware, onRequest));
} else {
pipeline.setMiddlewareFunction(i18nMiddleware);
}
pipeline.setMiddlewareFunction(sequence(i18nMiddleware, onRequest));
pipeline.onBeforeRenderRoute(i18nPipelineHook);
} else if (onRequest) {
} else {
pipeline.setMiddlewareFunction(onRequest);
}
if (!pageModulePromise) {
Expand Down Expand Up @@ -629,10 +634,11 @@ function getPrettyRouteName(route: RouteData): string {
* @param settings
* @param renderers
*/
export function createBuildManifest(
function createBuildManifest(
settings: AstroSettings,
internals: BuildInternals,
renderers: SSRLoadedRenderer[]
renderers: SSRLoadedRenderer[],
middleware: MiddlewareHandler
): SSRManifest {
let i18nManifest: SSRManifestI18n | undefined = undefined;
if (settings.config.i18n) {
Expand Down Expand Up @@ -660,5 +666,6 @@ export function createBuildManifest(
componentMetadata: internals.componentMetadata,
i18n: i18nManifest,
buildFormat: settings.config.build.format,
middleware
};
}
24 changes: 10 additions & 14 deletions packages/astro/src/core/build/plugins/plugin-manifest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,22 +42,18 @@ function vitePluginManifest(options: StaticBuildOptions, internals: BuildInterna
},
async load(id) {
if (id === RESOLVED_SSR_MANIFEST_VIRTUAL_MODULE_ID) {
const imports = [];
const contents = [];
const exports = [];
imports.push(
const imports = [
`import { deserializeManifest as _deserializeManifest } from 'astro/app'`,
`import { _privateSetManifestDontUseThis } from 'astro:ssr-manifest'`
);

contents.push(`
const manifest = _deserializeManifest('${manifestReplace}');
_privateSetManifestDontUseThis(manifest);
`);

exports.push('export { manifest }');

return `${imports.join('\n')}${contents.join('\n')}${exports.join('\n')}`;
];
const contents = [
`const manifest = _deserializeManifest('${manifestReplace}');`,
`_privateSetManifestDontUseThis(manifest);`
];
const exports = [
`export { manifest }`
];
return [...imports, ...contents, ...exports].join('\n');
}
},

Expand Down
21 changes: 0 additions & 21 deletions packages/astro/src/core/build/plugins/plugin-pages.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,10 @@
import { extname } from 'node:path';
import type { Plugin as VitePlugin } from 'vite';
import type { AstroSettings } from '../../../@types/astro.js';
import { routeIsRedirect } from '../../redirects/index.js';
import { addRollupInput } from '../add-rollup-input.js';
import { eachPageFromAllPages, type BuildInternals } from '../internal.js';
import type { AstroBuildPlugin } from '../plugin.js';
import type { StaticBuildOptions } from '../types.js';
import { MIDDLEWARE_MODULE_ID } from './plugin-middleware.js';
import { RENDERERS_MODULE_ID } from './plugin-renderers.js';
import { ASTRO_PAGE_EXTENSION_POST_PATTERN, getPathFromVirtualModulePageName } from './util.js';

Expand Down Expand Up @@ -37,7 +35,6 @@ export function getVirtualModulePageIdFromPath(path: string) {
function vitePluginPages(opts: StaticBuildOptions, internals: BuildInternals): VitePlugin {
return {
name: '@astro/plugin-build-pages',

options(options) {
if (opts.settings.config.output === 'static') {
const inputs = new Set<string>();
Expand All @@ -52,13 +49,11 @@ function vitePluginPages(opts: StaticBuildOptions, internals: BuildInternals): V
return addRollupInput(options, Array.from(inputs));
}
},

resolveId(id) {
if (id.startsWith(ASTRO_PAGE_MODULE_ID)) {
return '\0' + id;
}
},

async load(id) {
if (id.startsWith(ASTRO_PAGE_RESOLVED_MODULE_ID)) {
const imports: string[] = [];
Expand All @@ -74,15 +69,6 @@ function vitePluginPages(opts: StaticBuildOptions, internals: BuildInternals): V
imports.push(`import { renderers } from "${RENDERERS_MODULE_ID}";`);
exports.push(`export { renderers };`);

// The middleware should not be imported by the pages
if (shouldBundleMiddleware(opts.settings)) {
const middlewareModule = await this.resolve(MIDDLEWARE_MODULE_ID);
if (middlewareModule) {
imports.push(`import { onRequest } from "${middlewareModule.id}";`);
exports.push(`export { onRequest };`);
}
}

return `${imports.join('\n')}${exports.join('\n')}`;
}
}
Expand All @@ -91,13 +77,6 @@ function vitePluginPages(opts: StaticBuildOptions, internals: BuildInternals): V
};
}

export function shouldBundleMiddleware(settings: AstroSettings) {
if (settings.adapter?.adapterFeatures?.edgeMiddleware === true) {
return false;
}
return true;
}

export function pluginPages(opts: StaticBuildOptions, internals: BuildInternals): AstroBuildPlugin {
return {
targets: ['server'],
Expand Down
Loading

0 comments on commit dc75180

Please sign in to comment.