From 78404dfe1eb346723eefc183278b85f25485b419 Mon Sep 17 00:00:00 2001
From: Tee Ming <chewteeming01@gmail.com>
Date: Tue, 3 Dec 2024 18:30:10 +0800
Subject: [PATCH] fix: correctly match route groups preceding optional
 parameters (#13099)

fixes #13095

This PR changes the RegExp to ensure that routes such as /[[optional]]/(group) are treated the same as [[optional]]. Previously, /[[optional]]/(group) was being treated the same as / which is incorrect since it has an optional segment. It wasn't recognised as /[[optional]] because the RegExp didn't consider the route group preceding it (it only looked for the end of the string).
---
 .changeset/fifty-cars-lie.md                  |  5 +++
 .../sync/create_manifest_data/index.spec.js   | 42 +++++++++++++++++++
 .../core/sync/create_manifest_data/sort.js    |  3 +-
 .../[[optional]]/(group)/+page.svelte         |  0
 4 files changed, 49 insertions(+), 1 deletion(-)
 create mode 100644 .changeset/fifty-cars-lie.md
 create mode 100644 packages/kit/src/core/sync/create_manifest_data/test/samples/optional-group/[[optional]]/(group)/+page.svelte

diff --git a/.changeset/fifty-cars-lie.md b/.changeset/fifty-cars-lie.md
new file mode 100644
index 000000000000..cac282e057be
--- /dev/null
+++ b/.changeset/fifty-cars-lie.md
@@ -0,0 +1,5 @@
+---
+'@sveltejs/kit': patch
+---
+
+fix: correctly match route groups preceding optional parameters
diff --git a/packages/kit/src/core/sync/create_manifest_data/index.spec.js b/packages/kit/src/core/sync/create_manifest_data/index.spec.js
index 758206be45d5..2642e0c3cce0 100644
--- a/packages/kit/src/core/sync/create_manifest_data/index.spec.js
+++ b/packages/kit/src/core/sync/create_manifest_data/index.spec.js
@@ -412,6 +412,48 @@ test('nested optionals', () => {
 	]);
 });
 
+test('group preceding optional parameters', () => {
+	const { nodes, routes } = create('samples/optional-group');
+
+	expect(
+		nodes
+			.map(simplify_node)
+			// for some reason linux and windows have a different order, which is why
+			// we need sort the nodes using a sort function (doesn't work either without),
+			// resulting in the following expected node order
+			.sort((a, b) => a.component?.localeCompare(b.component ?? '') ?? 1)
+	).toEqual([
+		default_error,
+		default_layout,
+		{
+			component: 'samples/optional-group/[[optional]]/(group)/+page.svelte'
+		}
+	]);
+
+	expect(routes.map(simplify_route)).toEqual([
+		{
+			id: '/',
+			pattern: '/^/$/'
+		},
+		{
+			id: '/[[optional]]/(group)',
+			pattern: '/^(?:/([^/]+))?/?$/',
+			page: {
+				layouts: [0],
+				errors: [1],
+				// see above, linux/windows difference -> find the index dynamically
+				leaf: nodes.findIndex((node) =>
+					node.component?.includes('optional-group/[[optional]]/(group)')
+				)
+			}
+		},
+		{
+			id: '/[[optional]]',
+			pattern: '/^(?:/([^/]+))?/?$/'
+		}
+	]);
+});
+
 test('ignores files and directories with leading underscores', () => {
 	const { routes } = create('samples/hidden-underscore');
 
diff --git a/packages/kit/src/core/sync/create_manifest_data/sort.js b/packages/kit/src/core/sync/create_manifest_data/sort.js
index 9addd2c390c8..1629d146bf3e 100644
--- a/packages/kit/src/core/sync/create_manifest_data/sort.js
+++ b/packages/kit/src/core/sync/create_manifest_data/sort.js
@@ -136,7 +136,8 @@ function split_route_id(id) {
 	return get_route_segments(
 		id
 			// remove all [[optional]] parts unless they're at the very end
-			.replace(/\[\[[^\]]+\]\](?!$)/g, '')
+			// or it ends with a route group
+			.replace(/\[\[[^\]]+\]\](?!(?:\/\([^/]+\))*$)/g, '')
 	).filter(Boolean);
 }
 
diff --git a/packages/kit/src/core/sync/create_manifest_data/test/samples/optional-group/[[optional]]/(group)/+page.svelte b/packages/kit/src/core/sync/create_manifest_data/test/samples/optional-group/[[optional]]/(group)/+page.svelte
new file mode 100644
index 000000000000..e69de29bb2d1