VS Code Extension redirect is buggy

iterative / dvc.org

📖 DVC website and documentation

https://dvc.org

Apache License 2.0

335 stars 393 forks source link

VS Code Extension redirect is buggy #4248

Open jorgeorpinel opened 1 year ago

jorgeorpinel commented 1 year ago

Report

If you go to https://dvc.org/doc/install/ide-plugins#visual-studio-code and click on DVC Extension link

Install the DVC Extension for VS Code to use DVC right from your IDE!
you will be redirected to the 404 not found page and then redirected to the VS Code DVC extension page.
[x] the direct link is working fine

But it's the issue with the internal redirects as pointed by @rogermparent

Weird interaction with redirects that look like local pages, which are handled differently by the Gatsby's SPA via the Link component

[ ] Find a way to ensure that links to local page redirects are recognized as such and treated as external links

jorgeorpinel commented 1 year ago

Cc @iterative/websites

rogermparent commented 1 year ago

Weird interaction with redirects that look like local pages, which are handled differently by the Gatsby's SPA via the Link component. From what I can see, we'll have to:

Find a way to ensure that links to local page redirects are recognized as such and treated as external links (either automatically or manually, with pros/cons to each)
Possibly fight with gatsby-plugin-catch-links because it's kind of kludgy and invasive.

It's a bit late for me, but I can get a patch in early tomorrow if it's not already done.

jorgeorpinel commented 1 year ago

Sounds good. I don't think it's critical or anything, but it sure looks like a bug. Feel free to relabel though.

yathomasi commented 1 year ago

I remember fixing a similar issue mentioned for cml.dev.

https://github.com/iterative/cml.dev/issues/346#issuecomment-1283932384

But, I don't see that issue with dvc.org. Could it be a cache issue?

Firefox
Chrome
Also working fine with Safari

jorgeorpinel commented 1 year ago

Works OK after force-reload... 🤦🏼 Sorry for false alarm!

yathomasi commented 1 year ago

The issue seems genuine. Since it's not the direct link issue similar to cml.dev, so I guess it's not critical as mentioned. I have updated the description and assigned the priority accordingly.

rogermparent commented 1 year ago

I'm pretty sure it's just a generic behavior that'll happen using Gatsby's Link on things that aren't actually local links, but our wrapper Link component only reads the URL when making that decision. We have optOutPreRedirect on these markdown links, which would otherwise automatically transform the links into the absolute form, which would solve the problem but in a way I assume we also don't really want because we would never see the redirect in the links, and at that point just replacing the redirect with absolute links would work the same.

As far as gatsby-plugin-catch-links, it intercepts all links and applies the same behavior, so that's why I expect it to be a problem after the first issue is solved, but both problems need to be solved in order to properly use these redirects. I think I remember a long while ago dealing with this for cml.dev when there was a link for that in the sidebar, but things have changed a lot since then.