Skip to content

Commit

Permalink
fix: correctly match route groups preceding optional parameters (#13099)
Browse files Browse the repository at this point in the history
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).
  • Loading branch information
eltigerchino authored Dec 3, 2024
1 parent 38d65e3 commit 78404df
Show file tree
Hide file tree
Showing 4 changed files with 49 additions and 1 deletion.
5 changes: 5 additions & 0 deletions .changeset/fifty-cars-lie.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@sveltejs/kit': patch
---

fix: correctly match route groups preceding optional parameters
42 changes: 42 additions & 0 deletions packages/kit/src/core/sync/create_manifest_data/index.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -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');

Expand Down
3 changes: 2 additions & 1 deletion packages/kit/src/core/sync/create_manifest_data/sort.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand Down

0 comments on commit 78404df

Please sign in to comment.