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(vercel): ISR #9714

Merged
merged 21 commits into from
Feb 7, 2024
Merged
Show file tree
Hide file tree
Changes from 5 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
116 changes: 105 additions & 11 deletions packages/integrations/vercel/src/serverless/adapter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ const PACKAGE_NAME = '@astrojs/vercel/serverless';
* with the original path as the value of this header.
*/
export const ASTRO_PATH_HEADER = 'x-astro-path';
export const ASTRO_PATH_PARAM = 'x_astro_path';

/**
* The edge function calls the node server at /_render,
Expand All @@ -48,6 +49,11 @@ export const VERCEL_EDGE_MIDDLEWARE_FILE = 'vercel-edge-middleware';
export const NODE_PATH = '_render';
const MIDDLEWARE_PATH = '_middleware';

// This isn't documented by vercel anywhere, but unlike serverless
// and edge functions, isr functions are not passed the original path.
// Instead, we have to use $0 to refer to the regex match from "src".
const ISR_PATH = `/_isr?${ASTRO_PATH_PARAM}=$0`;

// https://vercel.com/docs/concepts/functions/serverless-functions/runtimes/node-js#node.js-version
const SUPPORTED_NODE_VERSIONS: Record<
string,
Expand Down Expand Up @@ -122,6 +128,33 @@ export interface VercelServerlessConfig {

/** The maximum duration (in seconds) that Serverless Functions can run before timing out. See the [Vercel documentation](https://vercel.com/docs/functions/serverless-functions/runtimes#maxduration) for the default and maximum limit for your account plan. */
maxDuration?: number;

/** Whether to cache on-demand rendered pages in the same way as static files. */
isr?: boolean | VercelISRConfig;
}

interface VercelISRConfig {
ematipico marked this conversation as resolved.
Show resolved Hide resolved
/**
* A random string that you create.
* Its presence in the `__prerender_bypass` cookie will result in fresh responses being served, bypassing the cache.
*
* By default, none.
*/
lilnasy marked this conversation as resolved.
Show resolved Hide resolved
bypassToken?: string;
natemoo-re marked this conversation as resolved.
Show resolved Hide resolved

/**
* Expiration time (in seconds) before the pages will be re-generated.
*
* By default, as long as the current deployment is in production.
Copy link
Member

@ematipico ematipico Jan 29, 2024

Choose a reason for hiding this comment

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

I don't understand this phrase. By default, what? It's not a boolean, so the default should be a number

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The default javascript value is false, but that just doesn't communicate anything.

Copy link
Member

@ematipico ematipico Jan 29, 2024

Choose a reason for hiding this comment

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

That's false because the parent object is optional, right? But if I pass an object, this field must have a default as stated in the docs. Which means I don't understand what's the default value in numbers of this option.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Does this help?

image

prerender-config.json

Copy link
Contributor Author

Choose a reason for hiding this comment

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

New lines 146-153.

*/
revalidate?: number;

/**
* Paths that will always be served fresh.
Copy link
Member

Choose a reason for hiding this comment

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

I would add a longer documentation here. From the implementation, I inferred that paths can be regex, but here I couldn't figure it out. I thought only strings were accepted.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That is correct. Only strings are accepted. Vercel would interpret the string as a regex but that's a vercel implementation detail. I will look into adding a processing step so that they really only ever act like strings.

*
* By default, none.
*/
exclude?: string[];
}

export default function vercelServerless({
Expand All @@ -135,6 +168,7 @@ export default function vercelServerless({
functionPerRoute = false,
edgeMiddleware = false,
maxDuration,
isr = false,
}: VercelServerlessConfig = {}): AstroIntegration {
if (maxDuration) {
if (typeof maxDuration !== 'number') {
Expand Down Expand Up @@ -224,6 +258,19 @@ export default function vercelServerless({
);
}
},
'astro:server:setup' ({ server }) {
// isr functions do not have access to search params, this middleware removes them for the dev mode
if (isr) {
const exclude_ = typeof isr === "object" ? isr.exclude ?? [] : [];
const exclude = exclude_.concat("/_image").map(ex => new RegExp(ex));
Copy link
Member

Choose a reason for hiding this comment

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

Instead of a regex expression, should we be a little accurate here? Looks like if it's passed ['/foo'], it could also match requests for /other/foo.

If we want to allow marking entire directories to exclude, maybe we can accept (string | RegExp)[] instead?

Copy link
Member

Choose a reason for hiding this comment

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

+1 from me too. Also, if a user passes a string, we should do an equality check, and not create a regex.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree we should be stricter.

It's a RegExp here because that's how vercel processes the routes file.

I'll look into making the exclude paths behave as literal paths.

server.middlewares.use(function removeIsrParams(req, _, next) {
const { pathname } = new URL(`https://example.com${req.url}`);
if (exclude.some(ex => ex.test(pathname))) return next();
req.url = pathname;
return next();
})
natemoo-re marked this conversation as resolved.
Show resolved Hide resolved
}
},
'astro:build:ssr': async ({ entryPoints, middlewareEntryPoint }) => {
_entryPoints = entryPoints;
_middlewareEntryPoint = middlewareEntryPoint;
Expand Down Expand Up @@ -282,27 +329,57 @@ export default function vercelServerless({
includeFiles,
excludeFiles,
maxDuration,
isr,
});
routeDefinitions.push({
src: route.pattern.source,
dest: func,
});
}
} else {
await createFunctionFolder({
functionName: NODE_PATH,
runtime,
entry: new URL(_serverEntry, _buildTempFolder),
config: _config,
logger,
NTF_CACHE,
includeFiles,
excludeFiles,
maxDuration,
});
if (isr) {
await createFunctionFolder({
functionName: '_isr',
runtime,
entry: new URL(_serverEntry, _buildTempFolder),
config: _config,
logger,
NTF_CACHE,
includeFiles,
excludeFiles,
maxDuration,
isr,
});
}
if (isr === false || (typeof isr === "object" && isr.exclude)) {
lilnasy marked this conversation as resolved.
Show resolved Hide resolved
await createFunctionFolder({
functionName: NODE_PATH,
runtime,
entry: new URL(_serverEntry, _buildTempFolder),
config: _config,
logger,
NTF_CACHE,
includeFiles,
excludeFiles,
maxDuration,
isr: false,
});
}
if (typeof isr === "object" && isr.exclude) {
lilnasy marked this conversation as resolved.
Show resolved Hide resolved
for (const route of isr.exclude) {
routeDefinitions.push({
src: route,
dest: NODE_PATH,
})
}
}
const dest = _middlewareEntryPoint ? MIDDLEWARE_PATH : NODE_PATH;
for (const route of routes) {
if (!route.prerender) routeDefinitions.push({ src: route.pattern.source, dest });
routeDefinitions.push({
src: route.pattern.source,
dest: isr ? ISR_PATH : NODE_PATH,
});
}
}
if (_middlewareEntryPoint) {
Expand Down Expand Up @@ -396,6 +473,7 @@ interface CreateFunctionFolderArgs {
includeFiles: URL[];
excludeFiles: URL[];
maxDuration: number | undefined;
isr: boolean | VercelISRConfig;
}

async function createFunctionFolder({
Expand All @@ -408,11 +486,13 @@ async function createFunctionFolder({
includeFiles,
excludeFiles,
maxDuration,
isr
}: CreateFunctionFolderArgs) {
// .vercel/output/functions/<name>.func/
const functionFolder = new URL(`./functions/${functionName}.func/`, config.outDir);
const packageJson = new URL(`./functions/${functionName}.func/package.json`, config.outDir);
const vcConfig = new URL(`./functions/${functionName}.func/.vc-config.json`, config.outDir);
const prerenderConfig = new URL(`./functions/${functionName}.prerender-config.json`, config.outDir)

// Copy necessary files (e.g. node_modules/)
const { handler } = await copyDependenciesToFunction(
Expand All @@ -439,6 +519,20 @@ async function createFunctionFolder({
maxDuration,
supportsResponseStreaming: true,
});

if (isr) {
const {
revalidate: expiration = false,
bypassToken = undefined,
} = typeof isr === "object" ? isr : {};
ematipico marked this conversation as resolved.
Show resolved Hide resolved
// https://vercel.com/docs/build-output-api/v3/primitives#prerender-configuration-file
await writeJson(prerenderConfig, {
expiration,
bypassToken,
allowQuery: [ASTRO_PATH_PARAM],
passQuery: true
});
}
}

function getRuntime(process: NodeJS.Process, logger: AstroIntegrationLogger): Runtime {
Expand Down
9 changes: 7 additions & 2 deletions packages/integrations/vercel/src/serverless/entrypoint.ts
Original file line number Diff line number Diff line change
@@ -1,16 +1,17 @@
import type { SSRManifest } from 'astro';
import { applyPolyfills, NodeApp } from 'astro/app/node';
import type { IncomingMessage, ServerResponse } from 'node:http';
import { ASTRO_PATH_HEADER, ASTRO_LOCALS_HEADER } from './adapter.js';
import { ASTRO_PATH_HEADER, ASTRO_PATH_PARAM, ASTRO_LOCALS_HEADER } from './adapter.js';

applyPolyfills();

export const createExports = (manifest: SSRManifest) => {
const app = new NodeApp(manifest);
const handler = async (req: IncomingMessage, res: ServerResponse) => {
const url = new URL(`https://example.com${req.url}`)
const clientAddress = req.headers['x-forwarded-for'] as string | undefined;
const localsHeader = req.headers[ASTRO_LOCALS_HEADER];
const realPath = req.headers[ASTRO_PATH_HEADER];
const realPath = req.headers[ASTRO_PATH_HEADER] ?? url.searchParams.get(ASTRO_PATH_PARAM);
if (typeof realPath === 'string') {
req.url = realPath;
}
Expand All @@ -26,3 +27,7 @@ export const createExports = (manifest: SSRManifest) => {

return { default: handler };
};

// HACK: prevent warning
// @astrojs-ssr-virtual-entry (22:23) "start" is not exported by "dist/serverless/entrypoint.js", imported by "@astrojs-ssr-virtual-entry".
export function start() {}
13 changes: 13 additions & 0 deletions packages/integrations/vercel/test/fixtures/isr/astro.config.mjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
import { defineConfig } from 'astro/config';
import vercel from '@astrojs/vercel/serverless';

export default defineConfig({
output: "server",
adapter: vercel({
isr: {
bypassToken: "1c9e601d-9943-4e7c-9575-005556d774a8",
revalidate: 120,
exclude: ["/two"]
}
})
});
9 changes: 9 additions & 0 deletions packages/integrations/vercel/test/fixtures/isr/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
{
"name": "@test/vercel-isr",
"version": "0.0.0",
"private": true,
"dependencies": {
"@astrojs/vercel": "workspace:*",
"astro": "workspace:*"
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
<html>
<head>
<title>One</title>
</head>
<body>
<h1>One</h1>
</body>
</html>
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
<html>
<head>
<title>Two</title>
</head>
<body>
<h1>Two</h1>
</body>
</html>
26 changes: 26 additions & 0 deletions packages/integrations/vercel/test/isr.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
import { loadFixture } from "./test-utils.js";
import { expect } from "chai";

describe("ISR", () => {
/** @type {import('./test-utils.js').Fixture} */
let fixture;

before(async () => {
fixture = await loadFixture({
root: "./fixtures/isr/",
});
await fixture.build();
});

it("generates expected prerender config", async () => {
const vcConfig = JSON.parse(
await fixture.readFile("../.vercel/output/functions/_isr.prerender-config.json")
);
expect(vcConfig).to.deep.include({
"expiration": 120,
"bypassToken": "1c9e601d-9943-4e7c-9575-005556d774a8",
"allowQuery": ["x_astro_path"],
"passQuery": true
})
})
})
9 changes: 9 additions & 0 deletions pnpm-lock.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading