Revisit how splats and repeated URL parameters work

[taion]

This is a follow-up to #2286 and #2421, and continues discussion from #2401.

The current API around splats and repeated URL parameters is pretty confusing and might lead to unexpected behavior. Two issues:

• Repeated URL parameters are automatically converted into arrays: https://github.com/rackt/react-router/blob/v1.0.0-rc3/modules/matchRoutes.js#L33-L37, which can be quite surprising if it happens unexpectedly
• Contra e.g. .gitignore usage, * and ** respectively mean non-greedy and greedy splats, rather than wildcards that match or don't match across path separators, leading to unintuitive behavior per the examples in [added] Greedy splat (**) #2401 (comment)

I think the best way to clean this up is:

• Get rid of the behavior for converting repeated parameters to arrays - just throw an exception if a parameter is repeated
• Make * and ** behave like in .gitignore - both should be greedy, but only * should match across path separators; this is significantly breaking, since it would mean that the recipe for a catch-all route would instead be <Route path="**" component={NotFound} />
• Stop binding splats to parameter values; for patterns like *.* for matching e.g. file names, instead use a custom rule per Customizable URL matching rules #2286

I think this would make things simpler, eliminate potentially confusing edge cases, and make behavior more consistent with other places.

[mjackson]

I don't think we need to make * and ** mean the same thing they mean in .gitignore. We're not trying to imitate git file-globbing syntax. We're just trying to match URL paths. Having a non-greedy * lets us do *.* really easily, and for the rare cases that someone wants * to be greedy (i.e. match across / characters), we use **. I guess I don't see what the problem is.

[taion]

What's unintuitive to me is that * will also match across / characters, but only sometimes. Per the OP to #2401, I don't think there's a good reason to have:

/*/c matches /you/are/okay/c
/*/c does not match /you/are/cool/c

I don't know that this would ever be what I want.

[mjackson]

You're right. The difference here is between greedy and non-greedy.

We need a non-greedy version of * in order to do stuff like /*/c. Otherwise the * would consume all the way to the end of that string. Apparently (per #2401) we also need a greedy version of *. That was the feature we were missing, and solved with **.

[taion]

I think the point I want to make is that the relevant distinction shouldn't be greedy vs non-greedy - it should be whether the splat matches path delimiters.

At https://github.com/rackt/react-router/blob/fbc109c0218508a301caae36469c8fc5d70de5fa/modules/PatternUtils.js#L26-L31, the most straightforward approach would be to have the pattern for * just be: ([^\/])* (i.e. everything except for path separators).

[taion]

I'm actually more bothered by the repeated parameter thing, BTW. If I have something like:

<Route path="/:foo" component={Parent}>
  <Route path=":foo" component={Child} />
</Route>

Then if I'm on /a, then Parent will receive params.foo as 'a', while if I go to /a/b, then Parent will receive params.foo as ['a', 'b'], assuming I'm not misunderstanding what the code does.

This example is contrived, but I'm afraid of users accidentally hitting this on e.g. deep, asynchronous routes where it's not as obvious that a route parameter name might be getting reused.

[itajaja]

I'm afraid of users accidentally hitting this

Maybe outputting a warning when a parameter name is used twice?

[taion]

I'd just throw while validating routes, once we have something in place for e.g. matching files with extensions so we don't need splats any more.

[DanielSWolf]

In our application, a given route component might be used as a sub-route in many different places, so it has no way of knowing in which parent routes it will be used. Consequently, there is always a chance of param name clashes between it and its parent routes.

Throwing an exception when a parameter name is used twice won't really help us. What I'd like to see instead is each route only receiving the params immediately following it, allowing params with identical names but different values to happily coexist.

To illustrate:

Let's say we have a route like a/:foo/:bar/b/:foo/:blubb and the path a/1/2/b/3/4. I'd like to get the following values:

• params = { foo: ['1', '3'], bar: '2', blubb: '4' } (I normally wouldn't use those)
• a.routeParams = { foo: '1', bar: '2' }
• b.routeParams = { foo: '3', blubb: '4' }

[taion]

I don't think we want to support that. It's quite nice to have params be globally consistent, and I don't think it'd be acceptable from a performance perspective to re-run matching just to get routeParams.

I do not think we should support repeated named params at all.

[taion]

We should do what Express does for splats (&c.) – use numerical indices.

[taion]

Closing this as part of #2286.

[taion]

The idea is – the choice of matching will affect splat handling anyway.