-
-
Notifications
You must be signed in to change notification settings - Fork 10.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Bug]: Link to adding to URL instead of replacing in v6.20, works in v6.18 #11074
Comments
We are seeing this too |
Confirmed that I am also having this error and have not found a solution. |
I assume this is happening inside a splat route, and if so I'm pretty sure this is duplicate of #11052. The tl;dr; is included in this comment but it seems a bunch of folks were relying on the buggy behavior so we are planning to take a closer look at the issue and fix. For now, I would recommend remaining on 6.18 if your apps relied no this behavior inside a splat route. If this is not inside a splat route, please provide a small repro and we can reopen this issue. |
I haven't heard of a "splat route" before this thread. TIL. I did confirm with v6.20.1 that it's reverted. Also confirmed that using I actually thought passing in just the ID was the intended use. I'll be sure to update things to use "./${id}" going forward. |
We're you rendering inside of a splat route in the end? Something like <Route path="something/*" element={<Element />} />
function Element() {
return <ListItemButton component={Link} to={thing.id}>Things</ListItemButton>
}
Do you happen to have a reproduction of this? I would have expected them to behave the same (in both 6.18.0 and 6.20.0). We'll be re-adding the fix we reverted in 6.20.1 behind a future flag shortly and will plan to include a longer explanation of expected usage. |
[EDIT]
They probably were behaving the same in 6.18 and 6.20 in regards to using "./". I had tried other things like "*" and "~", completely forgetting that the character should be ".". It wasn't until I read through the other issue that I was reminded of that. Can't keep my languages straight, apparently. ;) So we would have been fine just adding the "./" on our "to" paths in 6.20. Since we thought passing in the ID alone was intended use, it felt like a bug. |
That's what I'm trying to confirm - So I think you might have a different underlying issue entirely so I'd love to see a reproduction if you could create one to see if there's another issue at play here that we need to address. |
I created a CodeSandbox and couldn't reproduce the issue with a simple example. Went back to our app and did some digging. Found an inconsistency in the routing in our app. The specific route we were seeing this issue on WAS a splat route. That was the only spot we had a splat route and it wasn't necessary. I removed the "*" on that specific route and it's working even in v6.20.0. I should have caught this when I reviewed things for my last post. So you are safe. Not a different underlying issue. Just a bug we fixed in our app. :) |
Thanks for confirming! |
What version of React Router are you using?
6.20.0
Steps to Reproduce
Expected Behavior
Replaces the current ID in the URL with the ID provided to the "to" property.
Actual Behavior
This works as expected in v6.18.0, after upgrading to v6.20.0 instead of replacing the ID, it's adding another "/{ID}" to the URL, breaking navigation. I wasn't able to find an alternative string that worked.
The text was updated successfully, but these errors were encountered: