diff --git a/.changeset/brave-forms-work.md b/.changeset/brave-forms-work.md new file mode 100644 index 000000000000..587d070e1882 --- /dev/null +++ b/.changeset/brave-forms-work.md @@ -0,0 +1,5 @@ +--- +'@sveltejs/adapter-vercel': patch +--- + +fix: enable ISR only for `GET` and `HEAD` requests diff --git a/documentation/docs/25-build-and-deploy/90-adapter-vercel.md b/documentation/docs/25-build-and-deploy/90-adapter-vercel.md index eff04f2c9cfb..bf36ffe055e5 100644 --- a/documentation/docs/25-build-and-deploy/90-adapter-vercel.md +++ b/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 diff --git a/packages/adapter-vercel/index.js b/packages/adapter-vercel/index.js index 7cec5f02109a..7fabc3a2b870 100644 --- a/packages/adapter-vercel/index.js +++ b/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. diff --git a/packages/adapter-vercel/test/apps/basic/build.test.js b/packages/adapter-vercel/test/apps/basic/build.test.js index 63f7c3c3f383..528d5fc5e938 100644 --- a/packages/adapter-vercel/test/apps/basic/build.test.js +++ b/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`)); diff --git a/packages/adapter-vercel/test/apps/basic/src/routes/isr-endpoint/+page.server.ts b/packages/adapter-vercel/test/apps/basic/src/routes/isr-endpoint/+page.server.ts new file mode 100644 index 000000000000..ff47075319c8 --- /dev/null +++ b/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() + }; +} diff --git a/packages/adapter-vercel/test/apps/basic/src/routes/isr-endpoint/+page.svelte b/packages/adapter-vercel/test/apps/basic/src/routes/isr-endpoint/+page.svelte new file mode 100644 index 000000000000..ee605ea5a259 --- /dev/null +++ b/packages/adapter-vercel/test/apps/basic/src/routes/isr-endpoint/+page.svelte @@ -0,0 +1,6 @@ + + +

ISR Page with endpoint

+

{data.rendered_at}

diff --git a/packages/adapter-vercel/test/apps/basic/src/routes/isr-endpoint/+server.ts b/packages/adapter-vercel/test/apps/basic/src/routes/isr-endpoint/+server.ts new file mode 100644 index 000000000000..2f027a4ed966 --- /dev/null +++ b/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; diff --git a/packages/adapter-vercel/test/apps/basic/src/routes/isr/+page.server.ts b/packages/adapter-vercel/test/apps/basic/src/routes/isr/+page.server.ts index ff47075319c8..52b75a5f133f 100644 --- a/packages/adapter-vercel/test/apps/basic/src/routes/isr/+page.server.ts +++ b/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 }) +}; diff --git a/packages/adapter-vercel/test/apps/basic/src/routes/isr/+page.svelte b/packages/adapter-vercel/test/apps/basic/src/routes/isr/+page.svelte index 4fbeb20225e2..217774692c4f 100644 --- a/packages/adapter-vercel/test/apps/basic/src/routes/isr/+page.svelte +++ b/packages/adapter-vercel/test/apps/basic/src/routes/isr/+page.svelte @@ -1,6 +1,14 @@

ISR Page

{data.rendered_at}

+ +
+ +
+ +{#if form?.success} +

success

+{/if} diff --git a/packages/adapter-vercel/test/apps/basic/test/test.ts b/packages/adapter-vercel/test/apps/basic/test/test.ts index 2348823e6de3..47c0b552aa5c 100644 --- a/packages/adapter-vercel/test/apps/basic/test/test.ts +++ b/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'); diff --git a/packages/adapter-vercel/utils.js b/packages/adapter-vercel/utils.js index ed9253c8474a..173dabcc265c 100644 --- a/packages/adapter-vercel/utils.js +++ b/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 diff --git a/packages/adapter-vercel/utils.spec.js b/packages/adapter-vercel/utils.spec.js index d382bd5f6d00..4a89e991b808 100644 --- a/packages/adapter-vercel/utils.spec.js +++ b/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');