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

feat: expose middleware URL to integrations #7458

Merged
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
24 changes: 24 additions & 0 deletions .changeset/chilly-pants-fix.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
---
'astro': minor
---

Astro exposes the middleware file path to the integrations in the hook `astro:build:done`

```ts
// myIntegration.js
import type { AstroIntegration } from 'astro';
function integration(): AstroIntegration {
return {
name: "fancy-astro-integration",
hooks: {
'astro:build:done': ({ middlewareEntryPoint }) => {
if (middlewareEntryPoint) {
// do some operations
}
}
}
}
}
```

The `middlewareEntryPoint` is only defined if the user has created an Astro middleware.
1 change: 1 addition & 0 deletions packages/astro/src/@types/astro.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1869,6 +1869,7 @@ export interface AstroIntegration {
pages: { pathname: string }[];
dir: URL;
routes: RouteData[];
middlewareEntryPoint: URL | undefined;
}) => void | Promise<void>;
};
}
Expand Down
16 changes: 15 additions & 1 deletion packages/astro/src/core/build/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ import type {
} from '../../@types/astro';

import fs from 'fs';
import { fileURLToPath, pathToFileURL } from 'node:url';
import { join } from 'node:path';
import * as colors from 'kleur/colors';
import { performance } from 'perf_hooks';
import type * as vite from 'vite';
Expand All @@ -28,6 +30,7 @@ import { collectPagesData } from './page-data.js';
import { staticBuild, viteBuild } from './static-build.js';
import type { StaticBuildOptions } from './types.js';
import { getTimeStat } from './util.js';
import { isServerLikeOutput } from '../../prerender/utils.js';

export interface BuildOptions {
mode?: RuntimeMode;
Expand Down Expand Up @@ -185,13 +188,24 @@ class AstroBuilder {
});
debug('build', timerMessage('Additional assets copied', this.timer.assetsStart));

let middlewareEntryPoint = undefined;
// during the last phase of the build, the emitted code gets copied inside
// `dist/server/` folder, so we need to shred the old URL and make a new one again
if (internals.middlewareEntryPoint && isServerLikeOutput(this.settings.config)) {
const outDir = fileURLToPath(this.settings.config.outDir);
const middlewareRelativePath = fileURLToPath(internals.middlewareEntryPoint).slice(
fileURLToPath(this.settings.config.outDir).length
);
middlewareEntryPoint = pathToFileURL(join(outDir, 'server', middlewareRelativePath));
}

// You're done! Time to clean up.
await runHookBuildDone({
config: this.settings.config,
buildConfig,
pages: pageNames,
routes: Object.values(allPages).map((pd) => pd.route),
logging: this.logging,
middlewareEntryPoint,
});

if (this.logging.level && levels[this.logging.level] <= levels['info']) {
Expand Down
1 change: 1 addition & 0 deletions packages/astro/src/core/build/internal.ts
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,7 @@ export interface BuildInternals {
entryPoints: Map<RouteData, URL>;
ssrSplitEntryChunks: Map<string, Rollup.OutputChunk>;
componentMetadata: SSRResult['componentMetadata'];
middlewareEntryPoint?: URL;
}

/**
Expand Down
13 changes: 12 additions & 1 deletion packages/astro/src/core/build/plugins/plugin-middleware.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ const EMPTY_MIDDLEWARE = '\0empty-middleware';

export function vitePluginMiddleware(
opts: StaticBuildOptions,
_internals: BuildInternals
internals: BuildInternals
): VitePlugin {
return {
name: '@astro/plugin-middleware',
Expand Down Expand Up @@ -41,6 +41,17 @@ export function vitePluginMiddleware(
return 'export const onRequest = undefined';
}
},

writeBundle(_, bundle) {
for (const [chunkName, chunk] of Object.entries(bundle)) {
if (chunk.type === 'asset') {
continue;
}
if (chunk.fileName === 'middleware.mjs') {
Copy link
Member Author

@ematipico ematipico Jun 23, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will put this string in a shared variable in the following PRs

internals.middlewareEntryPoint = new URL(chunkName, opts.settings.config.outDir);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we combine outDir + "server" + chunkName here so we don't have to handle constructing the final middlewareEntryPoint again incore/build/index.ts? Otherwise this URL path would also point to a non existent path.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Technically, at this point of the execution, the path is correct. The files are moved later. If we decided to prefix "server" here, we would create a URL that point to a file that doesn't exist. If, in the future, we decide to expose this information to a hook that is called before the files are moved, we could incur an error.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's fair 👍 I'm mostly concerned that the final constructed path takes a lot of work to do so, which might not be great for perf. I won't let this block the PR though, I'm also fine with this for now.

}
}
},
};
}

Expand Down
21 changes: 12 additions & 9 deletions packages/astro/src/integrations/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -343,20 +343,22 @@ export async function runHookBuildGenerated({
}
}

type RunHookBuildDone = {
config: AstroConfig;
pages: string[];
routes: RouteData[];
logging: LogOptions;
middlewareEntryPoint: URL | undefined;
};

export async function runHookBuildDone({
config,
buildConfig,
pages,
routes,
logging,
}: {
config: AstroConfig;
buildConfig: BuildConfig;
pages: string[];
routes: RouteData[];
logging: LogOptions;
}) {
const dir = isServerLikeOutput(config) ? buildConfig.client : config.outDir;
middlewareEntryPoint,
}: RunHookBuildDone) {
const dir = isServerLikeOutput(config) ? config.build.client : config.outDir;
await fs.promises.mkdir(dir, { recursive: true });

for (const integration of config.integrations) {
Expand All @@ -367,6 +369,7 @@ export async function runHookBuildDone({
pages: pages.map((p) => ({ pathname: p })),
dir,
routes,
middlewareEntryPoint,
}),
logging,
});
Expand Down
23 changes: 22 additions & 1 deletion packages/astro/test/middleware.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ import { loadFixture } from './test-utils.js';
import { expect } from 'chai';
import * as cheerio from 'cheerio';
import testAdapter from './test-adapter.js';
import { fileURLToPath } from 'node:url';
import { readFileSync, existsSync } from 'node:fs';

describe('Middleware in DEV mode', () => {
/** @type {import('./test-utils').Fixture} */
Expand Down Expand Up @@ -104,12 +106,19 @@ describe('Middleware in PROD mode, SSG', () => {
describe('Middleware API in PROD mode, SSR', () => {
/** @type {import('./test-utils').Fixture} */
let fixture;
let middlewarePath;

before(async () => {
fixture = await loadFixture({
root: './fixtures/middleware-dev/',
output: 'server',
adapter: testAdapter({}),
adapter: testAdapter({
setEntryPoints(entryPointsOrMiddleware) {
if (entryPointsOrMiddleware instanceof URL) {
middlewarePath = entryPointsOrMiddleware;
}
},
}),
});
await fixture.build();
});
Expand Down Expand Up @@ -201,6 +210,18 @@ describe('Middleware API in PROD mode, SSR', () => {
const text = await response.text();
expect(text.includes('REDACTED')).to.be.true;
});

it('the integration should receive the path to the middleware', async () => {
expect(middlewarePath).to.not.be.undefined;
try {
const path = fileURLToPath(middlewarePath);
expect(existsSync(path)).to.be.true;
const content = readFileSync(fileURLToPath(middlewarePath), 'utf-8');
expect(content.length).to.be.greaterThan(0);
} catch (e) {
throw e;
}
});
});

describe('Middleware with tailwind', () => {
Expand Down
4 changes: 3 additions & 1 deletion packages/astro/test/ssr-split-manifest.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,9 @@ describe('astro:ssr-manifest, split', () => {
output: 'server',
adapter: testAdapter({
setEntryPoints(entries) {
entryPoints = entries;
if (entries) {
entryPoints = entries;
}
},
setRoutes(routes) {
currentRoutes = routes;
Expand Down
5 changes: 4 additions & 1 deletion packages/astro/test/test-adapter.js
Original file line number Diff line number Diff line change
Expand Up @@ -79,10 +79,13 @@ export default function (
setEntryPoints(entryPoints);
}
},
'astro:build:done': ({ routes }) => {
'astro:build:done': ({ routes, middlewareEntryPoint }) => {
if (setRoutes) {
setRoutes(routes);
}
if (setEntryPoints) {
setEntryPoints(middlewareEntryPoint);
}
},
},
};
Expand Down
12 changes: 0 additions & 12 deletions packages/integrations/vercel/src/serverless/adapter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -109,9 +109,6 @@ export default function vercelServerless({
// Remove temporary folder
await removeDir(buildTempFolder);

// Write middleware
await writeFile(new URL('./middleware.js', functionFolder), generateMiddlewareCode());

ematipico marked this conversation as resolved.
Show resolved Hide resolved
// Enable ESM
// https://aws.amazon.com/blogs/compute/using-node-js-es-modules-and-top-level-await-in-aws-lambda/
await writeJson(new URL(`./package.json`, functionFolder), {
Expand Down Expand Up @@ -149,12 +146,3 @@ function getRuntime() {
const major = version.split('.')[0]; // '16.5.0' --> '16'
return `nodejs${major}.x`;
}

function generateMiddlewareCode() {
return `
import {onRequest} from "somethere";
export default function middleware(request, context) {
return onRequest(request)
}
`;
}
5 changes: 0 additions & 5 deletions packages/integrations/vercel/test/edge-middleware.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,4 @@ describe('Serverless prerender', () => {
root: './fixtures/middleware/',
});
});

it('build successful', async () => {
await fixture.build();
expect(await fixture.readFile('../.vercel/output/static/index.html')).to.be.ok;
});
ematipico marked this conversation as resolved.
Show resolved Hide resolved
});