react-router: [Bug]: useResolvedPath breaking.

What version of React Router are you using?

6.20

Steps to Reproduce

Because of useResolvedPath is broken, now I am unable to deal with navigate inside the wildcard ‘/path/to/*’ route …

eg:

// route path is "/base/*"
const to = useResolvedPath('path/to');
const handle = () => navigate(to);
<Link to="path/to">path/to</Link>

Expected Behavior

It will always navigate to /base/path/to before useResolvedPath is broken.

Actual Behavior

It will navigate /base/path/to/path/to/path/to/… depends on my current path both navigate and <Link>.

About this issue

  • Original URL
  • State: closed
  • Created 7 months ago
  • Reactions: 26
  • Comments: 18 (7 by maintainers)

Commits related to this issue

Most upvoted comments

Please reopen this. You’ve published a breaking change in a minor version.

We take the stability of React Router seriously

Closing this issue proves the contrary. You can’t ignore the thousands of existing apps that your change have broken.

tl;dr; We’re going to revert this fix and put it behind a future flag.

@mjackson, @ryanflorence and I just talked through this in detail and we do think that this is buggy behavior in 6.18.0 and earlier. However, it’s obvious that a large number of folks were relying on this behavior so we are going to get this reverted and re-implement the fix behind a future flag so that you can opt-in at your convenience. We should have a patch release out this afternoon with the revert.

Along with the future flag PR (which I’ll link to in this issue when ready), we’ll include examples of where the original behavior is incorrect and how you can update your apps to work with the new behavior when you choose to opt-into the future flag.

Closing this issue proves the contrary. You can’t ignore the thousands of existing apps that your change have broken.

Please keep in mind that we can’t have any insight into how many folks were impacted by this fix at the time we closed this issue. Every bug fix is potentially a breaking change for somebody. As soon as we realized that this fix impacted a bunch of existing applications, we let y’all know we’d revisit the fix. So I would reiterate that we do take the stability of React Router seriously. That does not mean that there won’t ever be bugs 🙂

This is due to a bug fix that was included in 6.19.0 and called out in the release notes.

Relative path resolution without any leading . or .. is relative to the current location in the route hierarchy.

So if you are on a path /a/b/c and you resolve a path d/e via Link/navigate/useResolvedPath, the resulting resolved path should be /a/b/c/d/e since it is resolved from the current location.

Previously in splat routes, this was broken and was relative to the parent (if the splat was alone in the route path - path="*") or relative to only a portion of the current route path (path="parent/*"). This was incorrect and inconsistent with how relative routing works in React Router for static routes and dynamic param routes.

I put together small code sandbox to show the resulting consistency with the bug fix - https://codesandbox.io/p/sandbox/spring-resonance-kcgn6p?file=%2Fsrc%2Findex.js. If you set the react-router-dom version back to 6.18.0, you can see how the splat route is handled differently (and incorrectly) compared to other route types.

Another case that this shows itself (the original bug that was filed in Remix) is when using <Form action="."> on a splat route and the form does not submit to the current location but instead the parent location without the splat route which is also incorrect and results in a 405 error: https://codesandbox.io/p/sandbox/ecstatic-gagarin-fv76x7?file=%2Fsrc%2Findex.js

We apologize if your applications were relying on this buggy behavior. We take the stability of React Router seriously and did our best to call attention to this bug fix in the 6.19.0 release notes.

If you are using a splat route hierarchy such as <Route path="parent"><Route path="*"></Route>, then the best way to handle this is to add a .. to your path to make it resolve relative to the parent (without the splat value). If your splat is embedded in the path such as <Route path="parent/*"> and you want to “replace” the splat value, you can do that with a replacement such as:

let location = useLocation();
let params = useParams;
let newPath = location.pathname.replace(new RegExp(`${params["*"]}$`), 'new/path')

I can’t remember the details of it, so I’ll need to dig in, but I’m pretty sure we have always needed to handle * differently and we probably introduced a new bug when fixing the other.

I’m experiencing the same with Link and useNavigate. Everything seems to be working fine with version 6.19.0

Thanks a lot guys, you made the right decision. Apologies for the alarming tone, and let me take back my comment: you do care about your user base. Kudos!

Also, IMHO, alongside with #10579 this is the second critical, and easy to miss when upgrading dependencies, regression within ~5 months in what is supposed to be a quite straightforward and stable library. Not good.

The proposed solution to the problem wouldn’t work:

<BrowserRouter>
  <Routes>
    <Route path="dashboard">
      <Route path="*" element={<Dashboard />} />
    </Route>
  </Routes>
</BrowserRouter>

Since the “dashboard” path doesn’t have an element, it would render an <Outlet /> with null when you visit /dashboard.

You could either add another splat:

    <Route path="dashboard/*">
      <Route path="*" element={<Dashboard />} />
    </Route>

Or use an index route to make sure the Outlet isn’t empty: “Index routes render in their parent route’s outlet at the parent route’s path.”

    <Route path="dashboard">
      <Route index path="*" element={<Dashboard />} />
    </Route>

Splat routes are extremely useful and allows us to attach “mini apps” (features) that handle themselves, decide their own routing hierarchy, and don’t care about naming conflicts with other parts of the app.

As the changelog says:

This makes code splitting and compartmentalizing your app really easy. You could render the Dashboard as its own independent app, or embed it into your large app without making any changes to it.

Want a messaging system? Just plug it in: <Route path="/messages/*" element={<Messages />} />

This fix is now available behind the future.v7_relativeSplatPath flag in 6.21.0

The funny part for me is that I never used relative paths before and started using in the (short-lived) reverted version. When I updated this week everything broke 😿

This fix has been reverted in v6.20.1

You can’t ignore the thousands of existing apps that your change have broken.

Yep, Remix app broken in prod 🙋.

+1. useResolvedPath() is broken since v6.19.0, the issue persists in the latest v6.20.0, and it used to work fine in v6.18.0.

In my project I call useResolvedPath('*') when at the current location /some/path. Up to v6.18.0 it was resulting in

{
  "hash": "",
  "pathname": "/*",
  "search": ""
}

Since v6.19.0 it results in

{
  "hash": "",
  "pathname": "/some/path/*",
  "search": ""
}

Also, somehow, I haven’t experienced any issues in my other project with fewer and simpler route structure. I guess https://github.com/remix-run/react-router/pull/10983 is the primary suspect for the break.