[fixed] Match routes piece-by-piece

[taion]

Fixes #2355

This is mostly straightforward, except for adding some special-casing to check that we only match at path boundaries.

[taion]

This was a leftover from a direction I ended up abandoning, but it seems like it simplifies this function a bit to not use reduceRight.

[mjackson]

reduceRight uses the last params first, so subsequent params that are named the same thing appear after previous values in the params array.

[taion]

It seems simpler but equivalent to use reduce and push over reduceRight and unshift, though.

[taion]

Rebased.

[knowbody]

thanks!

[mjackson]

This PR makes me nervous. No tests? What are you trying to fix?

[mjackson]

@knowbody Please wait for me to review stuff like this before merging!

[knowbody]

I did it again, I stop merging lol 🙈

[mjackson]

If we have great tests around something, then merge away. But I think @taion made some changes to behavior that we don't actually have tests for.

I was demo-ing the router last night to a crowd in Vancouver, and some of the examples on master are broken. So we're in danger of regressing when we make major changes like this without any tests.

[taion]

@mjackson

Most of the code exercised here is covered by https://github.com/rackt/react-router/blob/master/modules/__tests__/matchRoutes-test.js. I didn't get a chance to add tests for a potentially new error case, which would be something like

<Route path="a">
  <Route path="b" />
</Route>

matching something like /ab, nor did I get a chance to add a test to actually cover #2355. I wanted to get this PR up and get some feedback before spending more time on this.

Are you on Discord or IRC? Might be easier to explain what this is trying to do on chat.

[mjackson]

Sorry, I'm busy right now. I can probably talk about this later tonight. For now, would you be ok reverting and opening another PR?

[mjackson]

BTW, just wanted to say thanks for working on this @taion. I don't mean to be unappreciative.

It's just been really tricky to get this right, hence the subtle bugs. Whenever we're making changes around this kind of thing we need to take the time to add stringent tests, preferably the kind that fail before we make the change to the code.

[taion]

I'm okay with reverting. I believe this code works, though - I'm putting together a PR right now that adds test cases that fail against the code prior to this PR, and verifies that the new code for splitting on path separators works.

[taion]

I just realized one consequence of this change - this now causes us to normalize duplicate slashes between route segments, e.g.

<Route path="/foo">
  <Route path="bar" />
</Route>

will now match e.g. /foo////////bar, whereas previously it would only have matched /foo/bar (but also e.g. /foo/bar///////). But #2425 is not consistent with that behavior (since it only normalizes slashes at the end).

Not sure what behavior is desirable here.