Refactor Anchor component to correctly detect all URLs #4090
Conversation
…xternal URL detection
|
|
@hichemfantar is attempting to deploy a commit to the Vercel Team on Vercel. A member of the Team first needs to authorize it. |
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
1 Skipped Deployment
|
|
the current approach breaks very easily when the user passes unexpected urls that the browser can easily resolve by itself. e.g. "/#heading" is correct, http is also viable, also what if the user passes a different protocol? you can make all these checks but there's always something you could miss and all of this is handled by the js api. use client is no issue because the component is still rendered server side use client doesn't disable ssr which is a common misconception
|
we don't want to check everything just I don't want to overcomplicate this component, I don't see convincing arguments |
|
There are many variations of urls that aren’t being checked. What if the component resolves incorrectly and the user can’t override the behavior? If you’re sure you don’t wanna go through with this then the best solution is to get rid of the fragile auto detection and allow a prop “external” that the user can pass. This way the user isn’t stuck when the inference fails |
|
Also what if it’s an http url that points to the same website? |
We can't handle every case, but if you want in your website - you can by passing custom |
That’s exactly the point of this pr, it allows you to detect every case you could however keep the existing implementation and add an extra prop “external” to override when needed |
The component is already complex because it has implicit undocumented behavior |
|
it was documented in blogpost https://the-guild.dev/blog/nextra-4#markdown-links-changes |
|
blogs aren't documentation however, blogs are generally for old users that are looking to migrate and news. new users won't know about this, especially since it doesn't show up in search this should be documented in the documentation itself under the link section. also migration guides should be included directly in the documentation. navigating to a blog on another website is bad ux. even i didn't know about this |
I can't do everything sorry, it's an Open Source project where I am a one maintainer which I maintain based on my free time |
|
That's what my PRs are for, I'm willing to work with you but I also need to know my efforts won't be blocked when the solution is proper and doesn't cause any regressions. there's nothing more frustrating than finding a limitation or hard coded behavior that can easily be made flexible. |
|
i have a lot of experience working with documentation generators, i'm willing to share my experience in form of PRs and help make nextra be on par with other established solutions |
Why:
correctly handles all types of links, local, anchor, external, http, https, and other kinds
use client doesn't disable ssr
What's being changed (if available, include any code snippets, screenshots, or gifs):
Check off the following:
deployment link in this PR's timeline (this link will be available after
opening the PR).