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

Remove need for #/ with HashRouter basename="" #8460

Closed
wants to merge 2 commits into from

Conversation

thejohnhoffer
Copy link

@thejohnhoffer thejohnhoffer commented Dec 9, 2021

Closes issue #8459.

I've created a codepen to show the results of this PR against the latest release.

I've created a hook called useNavigator, which wraps the router's navigator object. This wrapping only occurs when basename="". This wrapped navigator serializes each pathname without the leading /. The difference can only be seen when <HashRouter basename="">.

The difference is that paths are written to the URL like #abc instead of #/abc.

@remix-cla-bot
Copy link
Contributor

remix-cla-bot bot commented Dec 9, 2021

Thank you for signing the Contributor License Agreement. Let's get this merged! 🥳

1 similar comment
@remix-cla-bot
Copy link
Contributor

remix-cla-bot bot commented Dec 9, 2021

Thank you for signing the Contributor License Agreement. Let's get this merged! 🥳

@thejohnhoffer thejohnhoffer changed the title Remove need for #/ with HashRouter basename="/" Remove need for #/ with HashRouter basename="" Dec 9, 2021
@remix-cla-bot
Copy link
Contributor

remix-cla-bot bot commented Dec 9, 2021

Thank you for signing the Contributor License Agreement. Let's get this merged! 🥳

@remix-cla-bot
Copy link
Contributor

remix-cla-bot bot commented Dec 9, 2021

Thank you for signing the Contributor License Agreement. Let's get this merged! 🥳

@ryanflorence
Copy link
Member

Thanks but we'll keep the / :)

@thejohnhoffer
Copy link
Author

This is now possible with [email protected] and history PR #911.

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

Successfully merging this pull request may close these issues.

2 participants