fix: handle optional and rest routes for ISR correctly (#11928)

* fix: handle optional and rest routes for ISR correctly

When turning on ISR in adapter-vercel for a route ending in an optional or rest parameter, and that parameter not being set, the regex would yield wrong results. Consider a route like `foo/[...rest]` and the URL is just `/foo`, then the route config for Vercel would turn that into a ISR-request like `foo/[..rest]?pathname/%2Ffoo%2F` - note the trailing slash, which comes from the regex looking like `foo/$1/$2`. This trailing slash which wasn't there before can subsequently lead to the SvelteKit runtime issuing a trailing slash redirect (it thinks we're on /foo/ and issues a redirect to /foo), which is wrong an will lead to infinite redirects in the browser.
The fix is to account for optional and rest parameters when constructing the regex for the Vercel route config.

* Update packages/adapter-vercel/test/utils.spec.js

Co-authored-by: Ben McCann <322311+benmccann@users.noreply.github.com>

* Update packages/adapter-vercel/utils.js

Co-authored-by: Ben McCann <322311+benmccann@users.noreply.github.com>

* Update .changeset/dirty-points-drop.md

Co-authored-by: Ben McCann <322311+benmccann@users.noreply.github.com>

* Update packages/adapter-vercel/utils.js

Co-authored-by: Ben McCann <322311+benmccann@users.noreply.github.com>

* make the import unfollowable for typescript

---------

Co-authored-by: Ben McCann <322311+benmccann@users.noreply.github.com>

.changeset/dirty-points-drop.md

@@ -0,0 +1,5 @@
+---
+"@sveltejs/adapter-vercel": patch
+---
+
+fix: handle optional and rest routes for incremental static regeneration (ISR) correctly

packages/adapter-vercel/index.js

@@ -3,7 +3,7 @@ import path from 'node:path';
 import { fileURLToPath } from 'node:url';
 import { nodeFileTrace } from '@vercel/nft';
 import esbuild from 'esbuild';
-import { get_pathname } from './utils.js';
+import { get_pathname, pattern_to_src } from './utils.js';
 
 const name = '@sveltejs/adapter-vercel';
 const DEFAULT_FUNCTION_NAME = 'fn';
@@ -305,18 +305,7 @@ const plugin = function (defaults = {}) {
 				if (is_prerendered(route)) continue;
 
 				const pattern = route.pattern.toString();
-
-				let src = pattern
-					// remove leading / and trailing $/
-					.slice(1, -2)
-					// replace escaped \/ with /
-					.replace(/\\\//g, '/');
-
-				// replace the root route "^/" with "^/?"
-				if (src === '^/') {
-					src = '^/?';
-				}
-
+				const src = pattern_to_src(pattern);
 				const name = functions.get(pattern) ?? 'fn-0';
 
 				const isr = isr_config.get(route);

packages/adapter-vercel/test/utils.spec.js

@@ -1,5 +1,8 @@
 import { assert, test } from 'vitest';
-import { get_pathname } from '../utils.js';
+import { get_pathname, pattern_to_src } 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('../../kit/src/' + 'utils/routing.js');
 
 /**
  * @param {import('@sveltejs/kit').RouteDefinition<any>['segments']} segments
@@ -14,6 +17,16 @@ test('get_pathname for simple route', () => {
 	run_get_pathname_test([{ content: 'foo', dynamic: false, rest: false }], 'foo');
 });
 
+test('get_pathname for simple route with multiple segments', () => {
+	run_get_pathname_test(
+		[
+			{ content: 'foo', dynamic: false, rest: false },
+			{ content: 'bar', dynamic: false, rest: false }
+		],
+		'foo/bar'
+	);
+});
+
 test('get_pathname for route with parameters', () => {
 	run_get_pathname_test(
 		[
@@ -33,3 +46,87 @@ test('get_pathname for route with parameters within segment', () => {
 		'foo-$1/$2-buz'
 	);
 });
+
+test('get_pathname for route with optional parameters within segment', () => {
+	run_get_pathname_test(
+		[
+			{ content: 'foo-[[bar]]', dynamic: true, rest: false },
+			{ content: '[[baz]]-buz', dynamic: true, rest: false }
+		],
+		'foo-$1/$2-buz'
+	);
+});
+
+test('get_pathname for route with rest parameter', () => {
+	run_get_pathname_test(
+		[
+			{ content: 'foo', dynamic: false, rest: false },
+			{ content: '[[...rest]]', dynamic: true, rest: true }
+		],
+		'foo$1'
+	);
+});
+
+test('get_pathname for route with required and rest parameter', () => {
+	run_get_pathname_test(
+		[
+			{ content: '[foo]', dynamic: true, rest: false },
+			{ content: '[...rest]', dynamic: true, rest: true }
+		],
+		'$1$2'
+	);
+});
+
+test('get_pathname for route with required and optional parameter', () => {
+	run_get_pathname_test(
+		[
+			{ content: '[foo]', dynamic: true, rest: false },
+			{ content: '[[optional]]', dynamic: true, rest: true }
+		],
+		'$1$2'
+	);
+});
+
+test('get_pathname for route with required and optional parameter', () => {
+	run_get_pathname_test(
+		[
+			{ content: '[foo]', dynamic: true, rest: false },
+			{ content: '[[...rest]]', dynamic: true, rest: true },
+			{ content: 'bar', dynamic: false, rest: false }
+		],
+		'$1$2/bar'
+	);
+});
+
+/**
+ * @param {string} route_id
+ * @param {string} expected
+ */
+function run_pattern_to_src_test(route_id, expected) {
+	const { pattern } = parse_route_id(route_id);
+	assert.equal(pattern_to_src(pattern.toString()), expected);
+}
+
+test('pattern_to_src for simple route', () => {
+	run_pattern_to_src_test('/', '^/?');
+});
+
+test('pattern_to_src for route with parameters', () => {
+	run_pattern_to_src_test('/foo/[bar]', '^/foo/([^/]+?)/?');
+});
+
+test('pattern_to_src for route with optional parameters', () => {
+	run_pattern_to_src_test('/foo/[[bar]]', '^/foo(/[^/]+)?/?');
+});
+
+test('pattern_to_src for route with optional parameter in the middle', () => {
+	run_pattern_to_src_test('/foo/[[bar]]/baz', '^/foo(/[^/]+)?/baz/?');
+});
+
+test('pattern_to_src for route with rest parameter', () => {
+	run_pattern_to_src_test('/foo/[...bar]', '^/foo(/.*)?/?');
+});
+
+test('pattern_to_src for route with rest parameter in the middle', () => {
+	run_pattern_to_src_test('/foo/[...bar]/baz', '^/foo(/.*)?/baz/?');
+});

packages/adapter-vercel/utils.js

@@ -2,22 +2,68 @@
 export function get_pathname(route) {
 	let i = 1;
 
-	return route.segments
+	const pathname = route.segments
 		.map((segment) => {
 			if (!segment.dynamic) {
-				return segment.content;
+				return '/' + segment.content;
 			}
 
 			const parts = segment.content.split(/\[(.+?)\](?!\])/);
-			return parts
-				.map((content, j) => {
-					if (j % 2) {
-						return `$${i++}`;
-					} else {
-						return content;
-					}
-				})
-				.join('');
+			let result = '';
+
+			if (
+				parts.length === 3 &&
+				!parts[0] &&
+				!parts[2] &&
+				(parts[1].startsWith('...') || parts[1][0] === '[')
+			) {
+				// Special case: segment is a single optional or rest parameter.
+				// In that case we don't prepend a slash (also see comment in pattern_to_src).
+				result = `$${i++}`;
+			} else {
+				result =
+					'/' +
+					parts
+						.map((content, j) => {
+							if (j % 2) {
+								return `$${i++}`;
+							} else {
+								return content;
+							}
+						})
+						.join('');
+			}
+
+			return result;
 		})
-		.join('/');
+		.join('');
+
+	return pathname[0] === '/' ? pathname.slice(1) : pathname;
+}
+
+/**
+ * Adjusts the stringified route regex for Vercel's routing system
+ * @param {string} pattern stringified route regex
+ */
+export function pattern_to_src(pattern) {
+	let src = pattern
+		// remove leading / and trailing $/
+		.slice(1, -2)
+		// replace escaped \/ with /
+		.replace(/\\\//g, '/');
+
+	// replace the root route "^/" with "^/?"
+	if (src === '^/') {
+		src = '^/?';
+	}
+
+	// Move non-capturing groups that swallow slashes into their following capturing groups.
+	// This is necessary because during ISR we're using the regex to construct the __pathname
+	// query parameter: In case of a route like [required]/[...rest] we need to turn them
+	// into $1$2 and not $1/$2, because if [...rest] is empty, we don't want to have a trailing
+	// slash in the __pathname query parameter which wasn't there in the original URL, as that
+	// could result in a false trailing slash redirect in the SvelteKit runtime, leading to infinite redirects.
+	src = src.replace(/\(\?:\/\((.+?)\)\)/g, '(/$1)');
+
+	return src;
 }