Extend link checker to prevent PRs from breaking incoming links to Astro Docs

[Princesseuh]

Inside Astro's typing file, we have a lot of JSDoc comments that have links to the documentation. Those links are shown to users directly in their editors when they hover, complete etc stuff

Unfortunately, sometimes changes to the documentation break those links without us noticing. It'd be great if they could be added to Hippo's awesome link checker!

[hippotastic]

Thank you for creating this issue! I'm really glad you like our link checker and are suggesting additions to it! 😊

I understand and support the goal of ensuring that links from IntelliSense tooltips to Astro Docs are working. Our ecosystem and toolings are a big part of the overall developer experience and we should try our best to ensure that developers reach the correct part of the docs when they click on those links.

Here's my view on your idea of adding JSDoc comment links to link checker:

One of my primary goals when developing link checker was to provide docs authors with precise, clearly explained and actionable error messages. Whenever I show a red error cross on someone's PR, I want this to be a clear indicator that there is an actual error in the docs that must be fixed before this PR can be merged, and to provide the PR author with all the information they need to fix it. That's the reason why link checker adds an annotation at the precise source location that's causing the error, and even provides a suggested fix whenever possible.

If we were to scrape an "external" (non-docs) repo for docs URLs and then throw errors in link checker whenever any of those URLs were invalid, it would no longer be guaranteed that link checker errors are actually related to a given docs PR and are fixable by the PR author. A broken docs URL inserted into the Astro main repo would be able to cause all PR check runs in the Astro Docs repo to fail at once, without the PR authors being able to do anything about it. A red cross would no longer clearly indicate "you've made a mistake that needs to be fixed, and here's how to fix it", but it could also mean "there was an external change that doesn't have anything to do with your PR, but here's a red cross that you can ignore". I would like to avoid this situation. 😄

I have an alternate idea though how to solve this and hopefully still reach your goal:

Rather than introducing a dependency on another repo containing Astro's types, we could add checks that ensure that our docs URLs remain stable across PRs, and that there are redirects in place otherwise. Whenever it is detected that page URLs are being removed by a PR, we could request the PR author to provide a redirect from the removed page URL to a new one that now contains the content in question. If this redirect is missing, the error message for the PR author could be something like this:

You seem to have removed the page "/en/integrations/integrations" in your PR. As there may be external links pointing to this URL, please create a redirect from the removed page to an existing page. Please edit the file _redirects and add the following line to create a redirect: ...

I think that this should effectively prevent 404 errors when clicking on any JSDoc links. Once our docs have become more stable, we could also extend this check to URL fragments and provide a JavaScript-based URL hash rewriting if deemed necessary.

Do you think that this alternate idea would also solve your issue?

[Princesseuh]

Hey, thank you for your answer!

I agree this would fix the issue. It's even a good thing in general as we approach 1.0 to not break URLs too much

As we all know: Cool URLs don't change 😄

[sarah11918]

@hippotastic Is this still something you're interested in exploring? Would you put it as "would like to/will probably some work on this in the next 2-3 months?"

[hippotastic]

I still like this idea, but I'm not actively pursuing it and don't have plans to do so in the near future.

In case it helps to make an implementation through another developer more likely: This logic doesn't need to be a part of Link Checker at all. :)

Given recent developments (I'm working with another developer on externalizing Link Checker into a separate repository & GitHub action), I would even advise to keep it separate.

I think it should be a completely separate script that compares the live sitemap with the new one as proposed in my previous comment.

[sarah11918]

Given that this is not a link checker thing, and will not be a Hippo thing, AND since we know planned docs reorganization, which will involve breaking links, is coming in the next few months... I'm going to close this with "when we plan for moving stuff, let's make sure redirects are part of that plan." If we have a solid redirects plan, then that should also mean our editor links are covered.