Skip to content
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]: v6.4 behave different from v6.3 when the URL path has hash next to the param #9369

Closed
xdbobname opened this issue Sep 29, 2022 · 8 comments · Fixed by #9375
Closed
Labels

Comments

@xdbobname
Copy link

xdbobname commented Sep 29, 2022

What version of React Router are you using?

v6.4.0、 v6.4.1

Steps to Reproduce

  • <Route path="/components/:id/" element={mainLayout} />
  • <Link to="/comonents/title#Title-Title-label"}>
  • const param = useParams();
  • const {id} = param;

Expected Behavior

obviously the param 'id' should be title

Actual Behavior

the param 'id' is 'title#Title-Title-label'

@xdbobname xdbobname added the bug label Sep 29, 2022
@brophdawg11
Copy link
Contributor

Can you provide your route definitions so we know what "id" param you're referring to? That way we can make sure we fully grasp your specific issue. Or even better, a reproduction of this issue in code sandbox?

@xdbobname
Copy link
Author

emmm... I try a minify code in sandbox, but the bug didn't appear

@xdbobname
Copy link
Author

@xdbobname
Copy link
Author

okay, now the bug appear again, you just switch the version of react-router-dom to experience it.

@brophdawg11
Copy link
Contributor

Interesting - so this is technically not a valid to value, but worked in 6.3 due to window.location fixing the issue for you. If you choose to specify the individual portions of a path (pathname/search/hash), then you shouldn't be mixing hash values in pathname:

<Link to={{ pathname: "/components/title#Title-Title-className" }}>

You should either be splitting it out:

<Link to={{ pathname: "/components/title", hash: "#Title-Title-className" }}>

Or passing the to value as a string, and we'll parse it internally:

<Link to="/components/title#Title-Title-className">

In 6.3, we sent the concatenated URL (to.pathname + to.search + to.hash) to pushState first and then re-read from window.location in react-router-dom which has by that point properly split out pathname and hash.

6.4 inverts that ordering since we're fetching data before updating the URL, so we're using the pathname you sent verbatim and it's not getting "fixed" by pushState.

I'll look into whether we want to sanitize these in 6.4, but I would recommend changing to pass a string value to Link instead of a split out path object.

@brophdawg11
Copy link
Contributor

Instead of trying to sanitize these internally, we're going to detect these types of invalid paths and throw an error informing you to send in a correct path object or switch to a string (#9375)

@xdbobname
Copy link
Author

okay, thanks!

@brophdawg11
Copy link
Contributor

Just merged the linked PR - aiming to get a prerelease out today and a stable 6.4.2 next week 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants