Error on conflicting routes

[Rich-Harris]

Yet another follow-up to #7051, which can be considered an alternative to #7278. It reverts to the simpler algorithm initially present in that PR, but adds a check for routes that are in conflict with each other (for example x/[[y]] and [[y]]/x, which can't meaningfully coexist) so that the algorithm doesn't need to consider the additional cases added later in the PR.

The one case it doesn't handle is x/[...rest] vs [...rest]/x (or x/[[optional]] vs [...rest]/x), which is the exception to the rule that a route ending with a rest/optional parameters comes after an otherwise-equivalent route. I'm hopeful that we can solve that case without introducing a lot more complexity, but I haven't nailed it yet.

Another test that fails (both in this PR and in #7278) is foo/bar[...rest] vs foo/[...rest]bar.

Please don't delete this checklist! Before submitting the PR, please make sure you do the following:

• It's really useful if your PR references an issue where it is discussed ahead of time. In many cases, features are absent for a reason. For large changes, please create an RFC: https://github.com/sveltejs/rfcs
• This message body should clearly illustrate what problems it solves.
• Ideally, include a test that fails without this PR but passes with it.

Tests

• Run the tests with pnpm test and lint the project with pnpm lint and pnpm check

Changesets

• If your PR makes a change that should be noted in one or more packages' changelogs, generate a changeset by running pnpm changeset and following the prompts. All changesets should be patch until SvelteKit 1.0

[changeset-bot]

⚠️ No Changeset found

Latest commit: 304b493

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

[dummdidumm]

foo/bar[...rest] vs foo/[...rest]bar -> is that even allowed? Can rest params appear as part of a segment?

(if they can't, there's still foo/bar[[optional]] vs foo/[[optional]]bar, which .. should probably be a route conflict?)

[Rich-Harris]

is that even allowed? Can rest params appear as part of a segment?

It is, yeah — it's come up in the past for things like [...path].json — #4656. Less likely that you'd need a route like that since the big loading revamp a few months back, but still a legitimate use case.

foo/bar[[optional]] and foo/[[optional]]bar would be treated as a conflict. The way this PR detects conflicts is it normalizes routes by replacing [param] with <*> and [param=x] with <x> and [[param]] with <?*> and so on...

1. foo/bar<?*>
2. foo/<?*>bar

...then figures out all the permutations of those routes:

3. foo/bar
4. foo/bar<*>
5. foo/<*>bar
6. foo/bar

Since 3 (derived from 1) and 6 (derived from 2) are identical, those routes are identified as being incompatible.

[dummdidumm]

Looks good!

The only thing I'm not sure about - but relaxing this is always possible later in a non-breaking way - is the very strict conflict check.

Imagine you have a/[[b]]/c/[[d]] and a/c as routes. Under this algorithm, this is a conflict, but the user could say "yes I know that that's a conflict in one of these cases, but I don't want to duplicate routes into a/[b]/c/[d], a/[b]/c/ and a/c/[d] (because a/[b]/c/[[d]] and a/[[b]]/c/[d] would also be a conflict in one case).

I don't know how a algorithm that relaxes this and at the same time sorts these things correctly without bringing the complexity of that other PR back would look like (probably something like "if this optional route only has one nn-conflicting permutation left it's an error), so I'm giving my approval regardless of this wrinkle. You can decide if this overly strict check is reason enough to use more complex sorting logic after all like in the other PR or not. I haven't decided yet.

[Rich-Harris]

Ah, that's an interesting case. It seems like a fairly unlikely one, but if it did arise then it would be possible to work around, or we could relax the conflict detection as you say. Aside from that edge case I think having the conflict detection is useful regardless (e.g. it found a conflicting route in the test app that wasn't doing any work)