[Bug]: Optional segments break absolute child route paths

[fenok]

What version of React Router are you using?

6.6.2, also tried 6.7.0-pre.4

Steps to Reproduce

Consider the following routes:

<Routes>
  <Route path="/foo?" element={<Foo />}>
    <Route path="/foo?/bar" element={<Bar />} />
  </Route>
</Routes>

Expected Behavior

<Bar /> is rendered at either /foo/bar or /bar.

Actual Behavior

This fails with the following error: Absolute route path \"/bar\" nested under path \"/foo\" is not valid. An absolute child route path must start with the combined path of all its parent routes.

It looks like the only way to make it work is to switch to relative child route path ("bar").

Context: I build path patterns programmatically, in which case it's easier to use absolute patterns whenever possible.

[timdorr]

This is tangential, but why do you provide a path that is absolute if you know about the nesting structure from your route() definitions? It would seem easier to embrace the nested form of <Route> definitions and provide relative paths instead.

That being said, this may be a "by implicit design" case and won't be supported for optional segments. I didn't have much to do with implementation, so I'll leave the commentary to others.

[fenok]

@timdorr Thanks for the fast reply!

Indeed, I can do it right now, it's just a bit less convenient:

<Routes>
  <Route path={FOO.path} element={<Foo />}>
    // As opposed to FOO.BAR.path
    <Route path={FOO.$.BAR.relativePath} element={<Bar />} />
  </Route>
</Routes>

The API is optimized for absolute paths because they are generally safer, and there wasn't really a need for relative route paths before optional segments were introduced.

I just wanted to make sure that this is a known drawback of optional segments.

[timdorr]

Why not make path relative and provide a fullPath or absolutePath property?

Of course, I realize this is nitpicking your API and not addressing your problem.

[fenok]

The same routes are also used for URL (or rather, URL path) building. It works like this:

• FIRST.SECOND.THIRD.buildUrl({}) => /first/second/third
• FIRST.$.SECOND.THIRD.buildRelativeUrl({}) => second/third

For flexibility, $ effectively cuts everything to the left, so we have full control over which part of the path we use.
The same logic applies to path and relativePath.

Of course, I can adjust to changes in React Router and make relative child route paths more convenient, I just want to know whether I have to or not (i.e. whether it's a bug or deliberate design choice).

[brophdawg11]

Hm, yeah it makes sense this wouldn't work. I'm not quite sure if it's something we'll want to (or be able to "easily") fix with the current implementation, given some additional refactors we have planned for the matching algorithm.

The current implementation is a bit naive by design since we didn't want to mess with the ranking/scoring logic, so it basically explodes your route tree ahead of time just as you would have had to do previously to achieve this type of optional segment behavior.

So your tree above gets exploded to:

<Routes>
  <Route path="/foo" element={<Foo />}>
    <Route path="/foo/bar" element={<Bar />} />
    <Route path="/bar" element={<Bar />} />  {/* ❌ This is where your error happens */ }
  </Route>
  <Route path="" element={<Foo />}>
    <Route path="/foo/bar" element={<Bar />} />
    <Route path="/bar" element={<Bar />} />
  </Route>
</Routes>

So, I wouldn't call this a "bug" per-se given the implementation, sort of a design choice that doesn't play super well with nested absolute paths. I'm pretty focused on some other initiatives at the moment, but I'd be happy to look over a PR if you wanted to take a stab at a fix. It might be straightforward to filter out non-matching parent paths when we loop through explodeOptionalSegments. Otherwise this is something we could look into when we do the tree matching work.

[fenok]

@brophdawg11 Thanks for such a detailed response! I'll try to look into it, but I know very little about the inner workings of React Router, so no promises there. Anyway, I'm glad to hear that you're not opposed to the idea of fixing this, and it can totally wait for the new matching algorithm.

[github-actions]

This issue has been automatically marked stale because we haven't received a response from the original author in a while 🙈. This automation helps keep the issue tracker clean from issues that are not actionable. Please reach out if you have more information for us or you think this issue shouldn't be closed! 🙂 If you don't do so within 7 days, this issue will be automatically closed.

[fenok]

I think the issue is still relevant.