fix: ISR only for GET/HEAD requests

.changeset/brave-forms-work.md

@@ -0,0 +1,5 @@
+---
+'@sveltejs/adapter-vercel': patch
+---
+
+fix: enable ISR only for `GET` and `HEAD` requests

documentation/docs/25-build-and-deploy/90-adapter-vercel.md

@@ -51,7 +51,7 @@ You can set the following options:
 - `maxDuration`: [maximum execution duration](https://vercel.com/docs/functions/runtimes#max-duration) of the function. Defaults to `10` seconds for Hobby accounts, `15` for Pro and `900` for Enterprise
 - `isr`: configuration Incremental Static Regeneration, described below
 
-Configuration set in a layout applies to all the routes beneath that layout, unless overridden at a more granular level.
+Configuration set in a layout applies to all the routes beneath that layout, unless overridden at a more granular level. However, a `+page` and `+server` file in the same directory will always share the same configuration settings.
 
 If your functions need to access data in a specific region, it's recommended that they be deployed in the same region (or close to it) for optimal performance.
 
@@ -111,6 +111,8 @@ export const config = {
 
 > [!NOTE] Using ISR on a route with `export const prerender = true` will have no effect, since the route is prerendered at build time
 
+> [!NOTE] A route using ISR cannot have both a `+page` and a `+server` file with a `GET`, `HEAD` or `fallback` handler as the cached response will be used, skipping content negotiation.
+
 The `expiration` property is required; all others are optional. The properties are discussed in more detail below.
 
 ### expiration

packages/adapter-vercel/index.js

@@ -4,9 +4,15 @@ import process from 'node:process';
 import { fileURLToPath } from 'node:url';
 import { VERSION } from '@sveltejs/kit';
 import { nodeFileTrace } from '@vercel/nft';
-import { parse_isr_expiration, pattern_to_src, resolve_runtime } from './utils.js';
+import {
+	parse_isr_expiration,
+	pattern_to_src,
+	resolve_runtime,
+	validate_isr_route
+} from './utils.js';
 
 const INTERNAL = '![-]'; // this name is guaranteed not to conflict with user routes
+const ISR_BYPASS_METHODS = ['POST', 'PUT', 'PATCH', 'DELETE', 'OPTIONS', 'QUERY'];
 
 /** @type {typeof import('./index.js').default} **/
 const plugin = function (defaults = {}) {
@@ -117,6 +123,8 @@ const plugin = function (defaults = {}) {
 				}
 
 				if (config.isr) {
+					validate_isr_route(route);
+
 					const directory = path.relative('.', builder.config.files.routes + route.id);
 
 					if (config.isr.allowQuery?.includes('__pathname')) {
@@ -231,6 +239,8 @@ const plugin = function (defaults = {}) {
 			// their own path, so they must be routed before it to arrive with `__pathname`
 			/** @type {any[]} */
 			const static_isr_routes = [];
+			/** @type {any[]} */
+			const isr_bypass_routes = [];
 
 			for (const route of builder.routes) {
 				if (is_prerendered(route)) continue;
@@ -275,6 +285,12 @@ const plugin = function (defaults = {}) {
 					// since the function otherwise only sees its own path
 					const pathname = src.slice(1);
 
+					isr_bypass_routes.push({
+						src: `^(${pathname})(?:)$`,
+						methods: ISR_BYPASS_METHODS,
+						dest: `/${name}?__pathname=$1`
+					});
+
 					routes.push({
 						src: `^(${pathname})$`,
 						dest: `/${isr_name}?__pathname=$1`
@@ -326,7 +342,7 @@ const plugin = function (defaults = {}) {
 			}
 
 			const filesystem = static_config.routes.findIndex((route) => route.handle === 'filesystem');
-			static_config.routes.splice(filesystem, 0, ...static_isr_routes);
+			static_config.routes.splice(filesystem, 0, ...isr_bypass_routes, ...static_isr_routes);
 
 			if (builder.config.router.resolution === 'server') {
 				// Create a separate serverless function just for server-side route resolution.

packages/adapter-vercel/test/apps/basic/build.test.js

@@ -21,7 +21,7 @@ test('__data.json prerender config is not generated for server-only route', () =
 	assert.ok(!fs.existsSync(`${functions}/__data.json.prerender-config.json`));
 });
 
-/** @type {{ routes: Array<{ src?: string, dest?: string, handle?: string }> }} */
+/** @type {{ routes: Array<{ src?: string, dest?: string, handle?: string, methods?: string[] }> }} */
 const config = JSON.parse(fs.readFileSync(`${output}/config.json`, 'utf8'));
 const route_sources = config.routes.flatMap((route) =>
 	typeof route.src === 'string' ? [route.src] : []
@@ -62,6 +62,12 @@ test('dynamic ISR routes are matched after the filesystem', () => {
 	assert.ok(index > filesystem);
 });
 
+test('ISR bypass routes include every non-cacheable SvelteKit method', () => {
+	const route = config.routes.find((route) => route.src === '^(/isr-endpoint/?)(?:)$');
+
+	assert.deepEqual(route?.methods, ['POST', 'PUT', 'PATCH', 'DELETE', 'OPTIONS', 'QUERY']);
+});
+
 test('process.cwd() is traced from the project directory', () => {
 	const function_dir = fs.realpathSync(`${output}/functions/process-cwd.func`);
 	assert.ok(fs.existsSync(`${function_dir}/adapter-vercel/test/apps/basic/asset.txt`));

packages/adapter-vercel/test/apps/basic/src/routes/isr-endpoint/+page.server.ts

@@ -0,0 +1,11 @@
+export const config = {
+	isr: {
+		expiration: 60
+	}
+};
+
+export function load() {
+	return {
+		rendered_at: Date.now()
+	};
+}

packages/adapter-vercel/test/apps/basic/src/routes/isr-endpoint/+page.svelte

@@ -0,0 +1,6 @@
+<script lang="ts">
+	let { data } = $props();
+</script>
+
+<h1>ISR Page with endpoint</h1>
+<p id="rendered-at">{data.rendered_at}</p>

packages/adapter-vercel/test/apps/basic/src/routes/isr-endpoint/+server.ts

@@ -0,0 +1,12 @@
+import type { RequestHandler } from './$types';
+
+const respond_with_method: RequestHandler = async ({ request }) => {
+	return Response.json({ method: request.method, body: await request.text() });
+};
+
+export const POST = respond_with_method;
+export const PUT = respond_with_method;
+export const PATCH = respond_with_method;
+export const DELETE = respond_with_method;
+export const OPTIONS = respond_with_method;
+export const QUERY = respond_with_method;

packages/adapter-vercel/test/apps/basic/src/routes/isr/+page.server.ts

@@ -9,3 +9,7 @@ export function load() {
 		rendered_at: Date.now()
 	};
 }
+
+export const actions = {
+	default: () => ({ success: true })
+};

packages/adapter-vercel/test/apps/basic/src/routes/isr/+page.svelte

@@ -1,6 +1,14 @@
 <script lang="ts">
-	let { data } = $props();
+	let { data, form } = $props();
 </script>
 
 <h1>ISR Page</h1>
 <p id="rendered-at">{data.rendered_at}</p>
+
+<form method="POST">
+	<button>Submit</button>
+</form>
+
+{#if form?.success}
+	<p id="form-success">success</p>
+{/if}

packages/adapter-vercel/test/apps/basic/test/test.ts

@@ -32,6 +32,23 @@ test('$app/server read works', async ({ request }) => {
 	expect(text).toContain('Hello from $app/server read');
 });
 
+test('avoid serving ISR for non-GET/HEAD requests', async ({ request }) => {
+	for (const method of ['POST', 'PUT', 'PATCH', 'DELETE', 'OPTIONS', 'QUERY']) {
+		await test.step(method, async () => {
+			const body = method === 'OPTIONS' ? undefined : crypto.randomUUID();
+			const response = await request.fetch('/isr-endpoint', { method, data: body });
+			expect(response.ok()).toBe(true);
+			expect(await response.json()).toEqual({ method, body: body ?? '' });
+		});
+	}
+});
+
+test('form actions bypass ISR', async ({ page }) => {
+	await page.goto('/isr');
+	await page.getByRole('button', { name: 'Submit' }).click();
+	await expect(page.locator('#form-success')).toHaveText('success');
+});
+
 test('ISR route serves cached response', async ({ request }) => {
 	// first request warms the cache
 	const first = await request.get('/isr');

packages/adapter-vercel/utils.js

@@ -1,5 +1,21 @@
 import process from 'node:process';
 
+const CACHEABLE_METHODS = new Set(['GET', 'HEAD', '*']);
+
+/**
+ * @param {{ id: string, page: { methods: string[] }, api: { methods: string[] } }} route
+ */
+export function validate_isr_route(route) {
+	if (
+		route.page.methods.length > 0 &&
+		route.api.methods.some((method) => CACHEABLE_METHODS.has(method))
+	) {
+		throw new Error(
+			`The ${route.id} route cannot use ISR. It has a +page and a +server file that would return the same cached response for GET and HEAD requests. Either disable ISR or remove one of the files`
+		);
+	}
+}
+
 /**
  * Adjusts the stringified route regex for Vercel's routing system
  * @param {string} pattern stringified route regex

packages/adapter-vercel/utils.spec.js

@@ -1,5 +1,10 @@
 import { assert, test, describe } from 'vitest';
-import { parse_isr_expiration, pattern_to_src, resolve_runtime } from './utils.js';
+import {
+	parse_isr_expiration,
+	pattern_to_src,
+	resolve_runtime,
+	validate_isr_route
+} from './utils.js';
 
 // workaround so that TypeScript doesn't follow that import which makes it pick up that file and then error on missing import aliases
 const { parse_route_id } = await import(
@@ -80,6 +85,34 @@ describe('parse_isr_expiration', () => {
 	});
 });
 
+describe('validate_isr_route', () => {
+	const route = {
+		id: '/mixed',
+		page: { methods: ['GET'] },
+		api: { methods: [] }
+	};
+
+	test.each(['GET', 'HEAD', '*'])('rejects a page with a %s endpoint', (method) => {
+		assert.throws(
+			() => validate_isr_route({ ...route, api: { methods: [method] } }),
+			/cannot use ISR\. It has a \+page and a \+server file/
+		);
+	});
+
+	test.each(['POST', 'PUT', 'PATCH', 'DELETE', 'OPTIONS', 'QUERY'])(
+		'allows a page with a %s endpoint',
+		(method) => {
+			assert.doesNotThrow(() => validate_isr_route({ ...route, api: { methods: [method] } }));
+		}
+	);
+
+	test('allows an endpoint-only route with a fallback', () => {
+		assert.doesNotThrow(() =>
+			validate_isr_route({ ...route, page: { methods: [] }, api: { methods: ['*'] } })
+		);
+	});
+});
+
 describe('resolve_runtime', () => {
 	test('prefers override_key over default_key', () => {
 		const result = resolve_runtime('nodejs20.x', 'bun1.x');