-
Notifications
You must be signed in to change notification settings - Fork 10.3k
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
fix(gatsby-link): Adds the ability to not pass the pathPrefix on navigate #11625
Conversation
The automatically added pathPrefix to the `to` in the `navigate` function of the Link package makes it so that navigating to ids in a page is impossible without specifying the current page. For example, trying to do `navigate('#some-id')` would send you directly to the home page rather than add the hashtag to the URL. Since this behavior is probably intended as the base behavior, this changes add the ability to remove the pathPrefix rather than to add it, so keep backwards compatibility.
@Minivera few questions on this PR!
Thoughts? |
In my case, I ported an old parcel app to gatsby (A decision I do not regret!) and some of the navigation was done through handlers. For example, I have one menu that uses onClick handlers for the navigation and does some more actions before navigating. Some of my navigation behavior broke due to how navigation works here however.
I'd be more than happy to provide some URL processing to improve this very ugly solution! I might be wrong, but to my understanding, the reason to for the prefix is to prevent external urls right? So I guess we would have three types of URL:
I suppose case 1 and 2 are already handled, so we need to make sure that |
@Minivera I appreciate this PR, but I think I'm going to recommend a different approach for your use case. We internalize the However - in the case of a hash link the niceties we add on top of this base library are overhead, and not really wanted. For this use case I'd recommend just using the import { navigate } from '@reach/router'
navigate('#some-hash'); // obviously triggered by some dynamic action, e.g. onClick Will this work for you? If so--it might make sense to perhaps tweak our |
Also here's an example |
I sure could, I even did before moving to I'm too new to the gatsby world to say, but I feel like support for the hashlinks should be build-in the links and navigate function, since it is in reach 🤔. Hashlinks don't work that well in react usually (No auto scroll on refresh), but they work perfectly with gatsby thanks to SSR, so I'd love to help add support for routing with hashtags. But I'll leave it to your judgment, I can make a docs PR to specify that hashlinks don't work and close this one if necessary. (Also maybe make an issue to discuss this further?) |
@Minivera re:
Yeah - it'll be de-duped. We already depend on @reach/router for routing and other core utilities, so it's already being included.
Yeah - I see how we can improve this, for sure. But like I said, the Gatsby linking utility is designed for page and route linking (and includes optimizations and other functionality) for those use cases. Hash linking is not a supported use case for our Link component, unfortunately :/ So yeah--let's close this out (and thanks again!) and then feel free to open an issue or PR improved documentation so it's clearer what the expectations around using gatsby-link and linking utilities should be! Thanks! |
…ith the link package (#11909) ## Description The `<Link>` component and the `navigate` function do not allow the user to replace to a hash-link nor a query parameter while `@reach/router` allows it. This PR adds a small section in the Link API docs to let users know of possible workarounds this limitation. This was discussed in this PR: #11625
I'm confused. Are you saying that |
@acnebs I'm not saying that! By hash-linking, I mean only hash linking. If you're using I think the gatsby-link documentation helps with this. |
Packages/files changed
This changes the
gatsby-link
package (Should I update the version anywhere?) and the filessrc/index.js
andsrc/__tests__/index.test.js
in that package.Description
The automatically added pathPrefix to the
to
in thenavigate
function of the Link package makes it so that navigating to ids in a page is impossible without specifying the current page. For example, trying to donavigate('#some-id')
would send you directly to the home page rather than add the hashtag to the URL. At this moment, the only way to fix this is by using thewindow.___navigate
version.Since this behavior is probably intended as the base behavior, this changes adds the ability to remove the pathPrefix rather than to add it, so keep backwards compatibility.
How to test
Someone who would like to navigate to the url
something.com/page-1#some-id
can now use navigate like this:navigate('#some-id', { withoutPrefix: true })
. The newly added test should confirm this works.