fix: allow rest parameters to follow multiple optional - or not - parameters (#8761)

* fix: allow rest parameter to follow multiple optional - or not - parameters

* refactor: made it even simpler and patched comments

* refactor: consistent comments position

* chore: correct changeset

* minor stylistic tweak - avoid intermediate values unless needed for comprehension etc

---------

Co-authored-by: Aurele Nitoref <aurele.nitoref@icloud.com>
Co-authored-by: Rich Harris <hello@rich-harris.dev>

Author: oreilles

.changeset/fast-jokes-sing.md

@@ -0,0 +1,5 @@
+---
+'@sveltejs/kit': patch
+---
+
+fix: allow rest parameters to follow multiple optional - or not - parameters

packages/kit/src/utils/routing.js

@@ -126,57 +126,44 @@ export function exec(match, params, matchers) {
 
 	const values = match.slice(1);
 
-	let buffered = '';
+	let buffered = 0;
 
 	for (let i = 0; i < params.length; i += 1) {
 		const param = params[i];
-		let value = values[i];
+		const value = values[i - buffered];
 
+		// in the `[[a=b]]/.../[...rest]` case, if one or more optional parameters
+		// weren't matched, roll the skipped values into the rest
 		if (param.chained && param.rest && buffered) {
-			// in the `[[lang=lang]]/[...rest]` case, if `lang` didn't
-			// match, we roll it over into the rest value
-			value = value ? buffered + '/' + value : buffered;
-		}
+			result[param.name] = values
+				.slice(i - buffered, i + 1)
+				.filter((s) => s)
+				.join('/');
 
-		buffered = '';
+			buffered = 0;
+			continue;
+		}
 
+		// if `value` is undefined, it means this is an optional or rest parameter
 		if (value === undefined) {
-			// if `value` is undefined, it means this is
-			// an optional or rest parameter
 			if (param.rest) result[param.name] = '';
-		} else {
-			if (param.matcher && !matchers[param.matcher](value)) {
-				// in the `/[[a=b]]/[[c=d]]` case, if the value didn't satisfy the `b` matcher,
-				// try again with the next segment by shifting values rightwards
-				if (param.optional && param.chained) {
-					// @ts-expect-error TypeScript is... wrong
-					let j = values.indexOf(undefined, i);
-
-					if (j === -1) {
-						// we can't shift values any further, so hang on to this value
-						// so it can be rolled into a subsequent `[...rest]` param
-						const next = params[i + 1];
-						if (next?.rest && next.chained) {
-							buffered = value;
-						} else {
-							return;
-						}
-					}
-
-					while (j >= i) {
-						values[j] = values[j - 1];
-						j -= 1;
-					}
-
-					continue;
-				}
-
-				// otherwise, if the matcher returns `false`, the route did not match
-				return;
-			}
+			continue;
+		}
 
+		if (!param.matcher || matchers[param.matcher](value)) {
 			result[param.name] = value;
+			continue;
 		}
+
+		// in the `/[[a=b]]/...` case, if the value didn't satisfy the matcher,
+		// keep track of the number of skipped optional parameters and continue
+		if (param.optional && param.chained) {
+			buffered++;
+			continue;
+		}
+
+		// otherwise, if the matcher returns `false`, the route did not match
+		return;
 	}
 
 	if (buffered) return;

packages/kit/src/utils/routing.spec.js

@@ -153,6 +153,31 @@ const exec_tests = [
 		path: '/foo',
 		expected: { rest: 'foo' }
 	},
+	{
+		route: '/[[slug1=doesntmatch]]/[slug2]/[...rest]',
+		path: '/foo/bar/baz',
+		expected: { slug2: 'foo', rest: 'bar/baz' }
+	},
+	{
+		route: '/[[slug1=doesntmatch]]/[slug2]/[...rest]/baz',
+		path: '/foo/bar/baz',
+		expected: { slug2: 'foo', rest: 'bar' }
+	},
+	{
+		route: '/[[a=doesntmatch]]/[[b=doesntmatch]]/[[c=doesntmatch]]/[...d]',
+		path: '/a/b/c/d',
+		expected: { d: 'a/b/c/d' }
+	},
+	{
+		route: '/[[a=doesntmatch]]/[b]/[...c]/[d]/e',
+		path: '/foo/bar/baz/qux/e',
+		expected: { b: 'foo', c: 'bar/baz', d: 'qux' }
+	},
+	{
+		route: '/[[slug1=doesntmatch]]/[[slug2=doesntmatch]]/[...rest]',
+		path: '/foo/bar/baz',
+		expected: { rest: 'foo/bar/baz' }
+	},
 	{
 		route: '/[[slug1=doesntmatch]]/[[slug2=matches]]/[[slug3=doesntmatch]]/[...rest].json',
 		path: '/foo/bar/baz.json',