Allow chained optional parameters

.changeset/forty-icons-switch.md

@@ -0,0 +1,5 @@
+---
+'@sveltejs/kit': patch
+---
+
+Roll over non-matching optional parameters instead of 404ing

packages/kit/src/core/generate_manifest/index.js

@@ -99,18 +99,16 @@ export function generate_manifest({ build_data, relative_path, routes, format =
 			],
 			routes: [
 				${routes.map(route => {
-					route.types.forEach(type => {
-						if (type) matchers.add(type);
+					route.params.forEach(param => {
+						if (param.matcher) matchers.add(param.matcher);
 					});
 
 					if (!route.page && !route.endpoint) return;
 
 					return `{
 					id: ${s(route.id)},
 					pattern: ${route.pattern},
-					names: ${s(route.names)},
-					types: ${s(route.types)},
-					optional: ${s(route.optional)},
+					params: ${s(route.params)},
 					page: ${route.page ? `{ layouts: ${get_nodes(route.page.layouts)}, errors: ${get_nodes(route.page.errors)}, leaf: ${reindexed.get(route.page.leaf)} }` : 'null'},
 					endpoint: ${route.endpoint ? loader(join_relative(relative_path, resolve_symlinks(build_data.server.vite_manifest, route.endpoint.file).chunk.file)) : 'null'}
 				}`;

packages/kit/src/core/sync/create_manifest_data/index.js

@@ -24,6 +24,14 @@ export default function create_manifest_data({
 	const matchers = create_matchers(config, cwd);
 	const { nodes, routes } = create_routes_and_nodes(cwd, config, fallback);
 
+	for (const route of routes) {
+		for (const param of route.params) {
+			if (param.matcher && !matchers[param.matcher]) {
+				throw new Error(`No matcher found for parameter '${param.matcher}' in route ${route.id}`);
+			}
+		}
+	}
+
 	return {
 		assets,
 		matchers,
@@ -153,7 +161,7 @@ function create_routes_and_nodes(cwd, config, fallback) {
 				);
 			}
 
-			const { pattern, names, types, optional } = parse_route_id(id);
+			const { pattern, params } = parse_route_id(id);
 
 			/** @type {import('types').RouteData} */
 			const route = {
@@ -162,9 +170,7 @@ function create_routes_and_nodes(cwd, config, fallback) {
 
 				segment,
 				pattern,
-				names,
-				types,
-				optional,
+				params,
 
 				layout: null,
 				error: null,
@@ -273,9 +279,7 @@ function create_routes_and_nodes(cwd, config, fallback) {
 			id: '/',
 			segment: '',
 			pattern: /^$/,
-			names: [],
-			types: [],
-			optional: [],
+			params: [],
 			parent: null,
 			layout: null,
 			error: null,

packages/kit/src/core/sync/create_manifest_data/index.spec.js

@@ -87,7 +87,7 @@ test('creates routes', () => {
 		},
 		{
 			id: '/blog.json',
-			pattern: '/^/blog.json$/',
+			pattern: '/^/blog.json/?$/',
 			endpoint: { file: 'samples/basic/blog.json/+server.js' }
 		},
 		{
@@ -97,7 +97,7 @@ test('creates routes', () => {
 		},
 		{
 			id: '/blog/[slug].json',
-			pattern: '/^/blog/([^/]+?).json$/',
+			pattern: '/^/blog/([^/]+?).json/?$/',
 			endpoint: {
 				file: 'samples/basic/blog/[slug].json/+server.ts'
 			}
@@ -308,7 +308,7 @@ test('allows rest parameters inside segments', () => {
 		},
 		{
 			id: '/[...rest].json',
-			pattern: '/^/(.*?).json$/',
+			pattern: '/^/(.*?).json/?$/',
 			endpoint: {
 				file: 'samples/rest-prefix-suffix/[...rest].json/+server.js'
 			}
@@ -408,7 +408,7 @@ test('allows multiple slugs', () => {
 	assert.equal(routes.filter((route) => route.endpoint).map(simplify_route), [
 		{
 			id: '/[file].[ext]',
-			pattern: '/^/([^/]+?).([^/]+?)$/',
+			pattern: '/^/([^/]+?).([^/]+?)/?$/',
 			endpoint: {
 				file: 'samples/multiple-slugs/[file].[ext]/+server.js'
 			}
@@ -467,7 +467,7 @@ test('works with custom extensions', () => {
 		},
 		{
 			id: '/blog.json',
-			pattern: '/^/blog.json$/',
+			pattern: '/^/blog.json/?$/',
 			endpoint: {
 				file: 'samples/custom-extension/blog.json/+server.js'
 			}
@@ -479,7 +479,7 @@ test('works with custom extensions', () => {
 		},
 		{
 			id: '/blog/[slug].json',
-			pattern: '/^/blog/([^/]+?).json$/',
+			pattern: '/^/blog/([^/]+?).json/?$/',
 			endpoint: {
 				file: 'samples/custom-extension/blog/[slug].json/+server.js'
 			}

packages/kit/src/core/sync/write_types/index.js

@@ -195,8 +195,8 @@ function update_types(config, routes, route, to_delete = new Set()) {
 	// Makes sure a type is "repackaged" and therefore more readable
 	declarations.push('type Expand<T> = T extends infer O ? { [K in keyof O]: O[K] } : never;');
 	declarations.push(
-		`type RouteParams = { ${route.names
-			.map((param, idx) => `${param}${route.optional[idx] ? '?' : ''}: string`)
+		`type RouteParams = { ${route.params
+			.map((param) => `${param.name}${param.optional ? '?' : ''}: string`)
 			.join('; ')} }`
 	);
 
@@ -270,8 +270,8 @@ function update_types(config, routes, route, to_delete = new Set()) {
 			if (leaf) {
 				if (leaf.route.page) ids.push(`"${leaf.route.id}"`);
 
-				for (const name of leaf.route.names) {
-					layout_params.add(name);
+				for (const param of leaf.route.params) {
+					layout_params.add(param.name);
 				}
 
 				ensureProxies(page, leaf.proxies);

packages/kit/src/exports/vite/dev/index.js

@@ -156,9 +156,7 @@ export async function dev(vite, vite_config, svelte_config) {
 						return {
 							id: route.id,
 							pattern: route.pattern,
-							names: route.names,
-							types: route.types,
-							optional: route.optional,
+							params: route.params,
 							page: route.page,
 							endpoint: endpoint
 								? async () => {

packages/kit/src/runtime/client/parse.js

@@ -11,14 +11,14 @@ export function parse(nodes, server_loads, dictionary, matchers) {
 	const layouts_with_server_load = new Set(server_loads);
 
 	return Object.entries(dictionary).map(([id, [leaf, layouts, errors]]) => {
-		const { pattern, names, types, optional } = parse_route_id(id);
+		const { pattern, params } = parse_route_id(id);
 
 		const route = {
 			id,
 			/** @param {string} path */
 			exec: (path) => {
 				const match = pattern.exec(path);
-				if (match) return exec(match, { names, types, optional }, matchers);
+				if (match) return exec(match, params, matchers);
 			},
 			errors: [1, ...(errors || [])].map((n) => nodes[n]),
 			layouts: [0, ...(layouts || [])].map(create_layout_loader),

packages/kit/src/runtime/server/index.js

@@ -77,7 +77,7 @@ export async function respond(request, options, state) {
 			const match = candidate.pattern.exec(decoded);
 			if (!match) continue;
 
-			const matched = exec(match, candidate, matchers);
+			const matched = exec(match, candidate.params, matchers);
 			if (matched) {
 				route = candidate;
 				params = decode_params(matched);

packages/kit/src/utils/routing.js

@@ -5,44 +5,40 @@ const param_pattern = /^(\[)?(\.\.\.)?(\w+)(?:=(\w+))?(\])?$/;
  * @param {string} id
  */
 export function parse_route_id(id) {
-	/** @type {string[]} */
-	const names = [];
-
-	/** @type {string[]} */
-	const types = [];
-
-	/** @type {boolean[]} */
-	const optional = [];
-
-	// `/foo` should get an optional trailing slash, `/foo.json` should not
-	// const add_trailing_slash = !/\.[a-z]+$/.test(key);
-	let add_trailing_slash = true;
+	/** @type {import('types').RouteParam[]} */
+	const params = [];
 
 	const pattern =
 		id === '/'
 			? /^\/$/
 			: new RegExp(
 					`^${get_route_segments(id)
-						.map((segment, i, segments) => {
+						.map((segment) => {
 							// special case — /[...rest]/ could contain zero segments
 							const rest_match = /^\[\.\.\.(\w+)(?:=(\w+))?\]$/.exec(segment);
 							if (rest_match) {
-								names.push(rest_match[1]);
-								types.push(rest_match[2]);
-								optional.push(false);
+								params.push({
+									name: rest_match[1],
+									matcher: rest_match[2],
+									optional: false,
+									rest: true,
+									chained: true
+								});
 								return '(?:/(.*))?';
 							}
 							// special case — /[[optional]]/ could contain zero segments
 							const optional_match = /^\[\[(\w+)(?:=(\w+))?\]\]$/.exec(segment);
 							if (optional_match) {
-								names.push(optional_match[1]);
-								types.push(optional_match[2]);
-								optional.push(true);
+								params.push({
+									name: optional_match[1],
+									matcher: optional_match[2],
+									optional: true,
+									rest: false,
+									chained: true
+								});
 								return '(?:/([^/]+))?';
 							}
 
-							const is_last = i === segments.length - 1;
-
 							if (!segment) {
 								return;
 							}
@@ -73,29 +69,31 @@ export function parse_route_id(id) {
 											);
 										}
 
-										const [, is_optional, is_rest, name, type] = match;
+										const [, is_optional, is_rest, name, matcher] = match;
 										// It's assumed that the following invalid route id cases are already checked
 										// - unbalanced brackets
 										// - optional param following rest param
 
-										names.push(name);
-										types.push(type);
-										optional.push(!!is_optional);
+										params.push({
+											name,
+											matcher,
+											optional: !!is_optional,
+											rest: !!is_rest,
+											chained: is_rest ? i === 1 && parts[0] === '' : false
+										});
 										return is_rest ? '(.*?)' : is_optional ? '([^/]*)?' : '([^/]+?)';
 									}
 
-									if (is_last && content.includes('.')) add_trailing_slash = false;
-
 									return escape(content);
 								})
 								.join('');
 
 							return '/' + result;
 						})
-						.join('')}${add_trailing_slash ? '/?' : ''}$`
+						.join('')}/?$`
 			  );
 
-	return { pattern, names, types, optional };
+	return { pattern, params };
 }
 
 /**
@@ -119,35 +117,70 @@ export function get_route_segments(route) {
 
 /**
  * @param {RegExpMatchArray} match
- * @param {{
- *   names: string[];
- *   types: string[];
- *   optional: boolean[];
- * }} candidate
+ * @param {import('types').RouteParam[]} params
  * @param {Record<string, import('types').ParamMatcher>} matchers
  */
-export function exec(match, { names, types, optional }, matchers) {
+export function exec(match, params, matchers) {
 	/** @type {Record<string, string>} */
-	const params = {};
+	const result = {};
+
+	const values = match.slice(1);
 
-	for (let i = 0; i < names.length; i += 1) {
-		const name = names[i];
-		const type = types[i];
-		let value = match[i + 1];
+	let buffered = '';
 
-		if (value || !optional[i]) {
-			if (type) {
-				const matcher = matchers[type];
-				if (!matcher) throw new Error(`Missing "${type}" param matcher`); // TODO do this ahead of time?
+	for (let i = 0; i < params.length; i += 1) {
+		const param = params[i];
+		let value = values[i];
+
+		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;
+		}
 
-				if (!matcher(value)) return;
+		buffered = '';
+
+		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;
 			}
 
-			params[name] = value ?? '';
+			result[param.name] = value;
 		}
 	}
 
-	return params;
+	if (buffered) return;
+	return result;
 }
 
 /** @param {string} str */