feat: allow hyphenated matcher names (#16284)

closes #16277 

I don't really see any reason not to do this -- it doesn't even really
complicate the internal code.

---------

Co-authored-by: Rich Harris <rich.harris@vercel.com>
Co-authored-by: Rich Harris <richard.a.harris@gmail.com>
Co-authored-by: Ben McCann <322311+benmccann@users.noreply.github.com>
Co-authored-by: Tee Ming Chew <chewteeming01@gmail.com>

Author: 4 people

.changeset/allow-hyphenated-params-matchers.md

@@ -0,0 +1,5 @@
+---
+'@sveltejs/kit': minor
+---
+
+feat: allow hyphens in param and matcher names

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

@@ -142,7 +142,7 @@ function create_routes_and_nodes(cwd, config, fallback) {
 				throw new Error(`Route ${id} should be renamed to ${id.replace(/#/g, '[x+23]')}`);
 			}
 
-			if (/\[\.\.\.\w+\]\/\[\[/.test(id)) {
+			if (/\[\.\.\.[\w-]+\]\/\[\[/.test(id)) {
 				throw new Error(
 					`Invalid route ${id} — an [[optional]] route segment cannot follow a [...rest] route segment`
 				);

packages/kit/src/core/sync/write_non_ambient.js

@@ -209,7 +209,7 @@ function generate_app_types(manifest_data, config) {
 		if (route.params.length > 0) {
 			const params = route.params.map((p) => {
 				const type = get_matcher_type(p.matcher);
-				return `${p.name}${p.optional ? '?:' : ':'} ${type}${p.optional ? ' | undefined' : ''}`;
+				return `${/^\w+$/.test(p.name) ? p.name : `'${p.name}'`}${p.optional ? '?:' : ':'} ${type}${p.optional ? ' | undefined' : ''}`;
 			});
 			const route_type = `${s(route.id)}: { ${params.join('; ')} }`;
 
@@ -230,7 +230,7 @@ function generate_app_types(manifest_data, config) {
 			const params = Array.from(layout_params)
 				.map(([name, { optional, matchers }]) => {
 					const type = get_matchers_type(matchers);
-					return `${name}${optional ? '?:' : ':'} ${type}${optional ? ' | undefined' : ''}`;
+					return `${/^\w+$/.test(name) ? name : `'${name}'`}${optional ? '?:' : ':'} ${type}${optional ? ' | undefined' : ''}`;
 				})
 				.join('; ');
 

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

@@ -607,7 +607,7 @@ function generate_params_type(params, outdir, config) {
 	return `{ ${params
 		.map(
 			(param) =>
-				`${param.name}${param.optional ? '?' : ''}: ${
+				`${/^\w+$/.test(param.name) ? param.name : `'${param.name}'`}${param.optional ? '?' : ''}: ${
 					param.matcher
 						? `import('@sveltejs/kit').MatcherParam<(typeof import('${params_import}').params)[${JSON.stringify(param.matcher)}]>`
 						: 'string'

packages/kit/src/core/sync/write_types/test/param-type-inference/optional/[[optional-hyphen-param=hyphenated-matcher]]/+page.js

@@ -0,0 +1,11 @@
+/* eslint-disable */
+
+/** @type {import('../../.svelte-kit/types/optional/[[optional-hyphen-param=hyphenated-matcher]]/$types').PageLoad} */
+export function load({ params }) {
+	if (params['optional-hyphen-param']) {
+		/** @type {"a" | "b"} */
+		let a;
+		a = params['optional-hyphen-param'];
+		return { a };
+	}
+}

packages/kit/src/core/sync/write_types/test/param-type-inference/params.js

@@ -6,6 +6,10 @@ export const params = defineParams({
 		if (!['a', 'b'].includes(param)) return;
 		return /** @type {'a' | 'b'} */ (param);
 	},
+	'hyphenated-matcher': (param) => {
+		if (!['a', 'b'].includes(param)) return;
+		return /** @type {'a' | 'b'} */ (param);
+	},
 	boolean: () => /** @type {boolean} */ (true),
 	number: v.pipe(v.string(), v.toNumber())
 });

packages/kit/src/core/sync/write_types/test/param-type-inference/required/[hyphen-param=hyphenated-matcher]/+page.js

@@ -0,0 +1,8 @@
+/* eslint-disable */
+
+/** @type {import('../../.svelte-kit/types/required/[hyphen-param=hyphenated-matcher]/$types').PageLoad} */
+export function load({ params }) {
+	/** @type {"a" | "b"} */
+	let a;
+	a = params['hyphen-param'];
+}

packages/kit/src/utils/routing.js

@@ -1,6 +1,6 @@
 import { BROWSER } from 'esm-env';
 
-const param_pattern = /^(\[)?(\.\.\.)?(\w+)(?:=(\w+))?(\])?$/;
+const param_pattern = /^(\[)?(\.\.\.)?([\w-]+)(?:=([\w-]+))?(\])?$/;
 
 const root_group_pattern = /^\/\((?:[^)]+)\)$/;
 
@@ -19,7 +19,7 @@ export function parse_route_id(id) {
 					`^${get_route_segments(id)
 						.map((segment) => {
 							// special case — /[...rest]/ could contain zero segments
-							const rest_match = /^\[\.\.\.(\w+)(?:=(\w+))?\]$/.exec(segment);
+							const rest_match = /^\[\.\.\.([\w-]+)(?:=([\w-]+))?\]$/.exec(segment);
 							if (rest_match) {
 								params.push({
 									name: rest_match[1],
@@ -31,7 +31,7 @@ export function parse_route_id(id) {
 								return '(?:/([^]*))?';
 							}
 							// special case — /[[optional]]/ could contain zero segments
-							const optional_match = /^\[\[(\w+)(?:=(\w+))?\]\]$/.exec(segment);
+							const optional_match = /^\[\[([\w-]+)(?:=([\w-]+))?\]\]$/.exec(segment);
 							if (optional_match) {
 								params.push({
 									name: optional_match[1],
@@ -72,7 +72,7 @@ export function parse_route_id(id) {
 										const match = /** @type {RegExpExecArray} */ (param_pattern.exec(content));
 										if (!BROWSER && !match) {
 											throw new Error(
-												`Invalid param: ${content}. Params and matcher names can only have underscores and alphanumeric characters.`
+												`Invalid param: ${content}. Params and matcher names can only have underscores, hyphens, and alphanumeric characters.`
 											);
 										}
 
@@ -103,7 +103,7 @@ export function parse_route_id(id) {
 	return { pattern, params };
 }
 
-const optional_param_regex = /\/\[\[\w+?(?:=\w+)?\]\]/;
+const optional_param_regex = /\/\[\[[\w-]+?(?:=[\w-]+)?\]\]/;
 
 /**
  * Removes optional params from a route ID.
@@ -260,7 +260,7 @@ function escape(str) {
 	);
 }
 
-const basic_param_pattern = /\[(\[)?(\.\.\.)?(\w+?)(?:=(\w+))?\]\]?/g;
+const basic_param_pattern = /\[(\[)?(\.\.\.)?([\w-]+?)(?:=([\w-]+))?\]\]?/g;
 
 /**
  * Populate a route ID with params to resolve a pathname.

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

@@ -55,6 +55,48 @@ describe('parse_route_id', () => {
 		'/@-symbol/[id]': {
 			pattern: /^\/@-symbol\/([^/]+?)\/?$/,
 			params: [{ name: 'id', matcher: undefined, optional: false, rest: false, chained: false }]
+		},
+		'/blog/[page-slug]': {
+			pattern: /^\/blog\/([^/]+?)\/?$/,
+			params: [
+				{ name: 'page-slug', matcher: undefined, optional: false, rest: false, chained: false }
+			]
+		},
+		'/blog/[page-slug=positive-integer]': {
+			pattern: /^\/blog\/([^/]+?)\/?$/,
+			params: [
+				{
+					name: 'page-slug',
+					matcher: 'positive-integer',
+					optional: false,
+					rest: false,
+					chained: false
+				}
+			]
+		},
+		'/blog/[[page-slug=positive-integer]]/sub': {
+			pattern: /^\/blog(?:\/([^/]+))?\/sub\/?$/,
+			params: [
+				{
+					name: 'page-slug',
+					matcher: 'positive-integer',
+					optional: true,
+					rest: false,
+					chained: true
+				}
+			]
+		},
+		'/[...catch-all]': {
+			pattern: /^(?:\/([^]*))?\/?$/,
+			params: [
+				{ name: 'catch-all', matcher: undefined, optional: false, rest: true, chained: true }
+			]
+		},
+		'/[...catch-all=some-matcher]': {
+			pattern: /^(?:\/([^]*))?\/?$/,
+			params: [
+				{ name: 'catch-all', matcher: 'some-matcher', optional: false, rest: true, chained: true }
+			]
 		}
 	};
 
@@ -404,6 +446,21 @@ describe('resolve_route', () => {
 			route: '/blog/[one]/[...two]-not-three/',
 			params: { one: 'one', two: 'two/2' },
 			expected: '/blog/one/two/2-not-three/'
+		},
+		{
+			route: '/blog/[page-slug]',
+			params: { 'page-slug': 'hello' },
+			expected: '/blog/hello'
+		},
+		{
+			route: '/blog/[page-slug=positive-integer]',
+			params: { 'page-slug': '42' },
+			expected: '/blog/42'
+		},
+		{
+			route: '/[...catch-all=some-matcher]',
+			params: { 'catch-all': 'a/b' },
+			expected: '/a/b'
 		}
 	];
 
@@ -420,6 +477,12 @@ describe('resolve_route', () => {
 		);
 	});
 
+	test('resolvePath errors on missing params for required param with hyphenated name', () => {
+		expect(() => resolve_route('/blog/[page-slug]', {})).toThrow(
+			"Missing parameter 'page-slug' in route /blog/[page-slug]"
+		);
+	});
+
 	test('resolvePath errors on params values starting or ending with slashes', () => {
 		assert.throws(
 			() => resolve_route('/blog/[one]/[two]', { one: 'one', two: '/two' }),
@@ -532,6 +595,22 @@ describe('find_route', () => {
 		);
 	});
 
+	test('respects matchers with hyphenated names', () => {
+		const routes = [create_route('/blog/[slug=positive-integer]'), create_route('/blog/[slug]')];
+		/** @type {import('@sveltejs/kit').ParamDefinition} */
+		const positive_integer = (param) => (/^\d+$/.test(param) ? param : undefined);
+		const matchers = defineParams({ 'positive-integer': positive_integer });
+
+		// "42" matches the positive-integer matcher
+		const result1 = find_route('/blog/42', routes, matchers);
+		assert.equal(result1?.route.id, '/blog/[slug=positive-integer]');
+		assert.deepEqual(result1?.params, { slug: '42' });
+
+		// "hello" doesn't match, falls through to [slug]
+		const result2 = find_route('/blog/hello', routes, matchers);
+		assert.equal(result2?.route.id, '/blog/[slug]');
+	});
+
 	test('decodes params', () => {
 		const routes = [create_route('/blog/[slug]')];