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

Checks links rot with CI + have permalink #2465

Closed
imagoiq opened this issue Jan 11, 2024 · 7 comments · Fixed by #2603
Closed

Checks links rot with CI + have permalink #2465

imagoiq opened this issue Jan 11, 2024 · 7 comments · Fixed by #2603
Assignees
Labels
📦 documentation Related to the @swisspost/design-system-documentation package research

Comments

@imagoiq
Copy link
Contributor

imagoiq commented Jan 11, 2024

From time to time, we have rot links on the documentation. To prevent that, we could have a workflow in the CI pipeline to checks all links. This workflow would not need to run in every PR, but daily for example because external can break at anytime.

Impediment:
Storybook is dynamically generated and needs more time to fully load a page. Does a link checker support this situation?

Some options to analyse:
https://github.com/lycheeverse/lychee
https://github.com/untitaker/hyperlink#options
https://github.com/marketplace/actions/check-links-with-linkcheck

@imagoiq
Copy link
Contributor Author

imagoiq commented Jan 15, 2024

Two issues:

  1. Most (if not all) of the link checkers don't support websites with dynamically rendered content
  2. Storybook doesn't output regular 404 when a story does not exist (e.g. https://next.design-system.post.ch/?path=/docs/foundations-layout-fake--docs)

The first issue could be resolved by using a checker which looks at the source: the MDX files. But then we need to parse links to make a difference between external and internal links, and the check for internal links would still need to be done dynamically. We could use a headless browser, but it makes all the things more complex and prone to error and flakiness…

@imagoiq imagoiq linked a pull request Jan 15, 2024 that will close this issue
@oliverschuerch
Copy link
Contributor

Please check first, if there is a plugin which allows us to use something like permalinks for our stories.

@alizedebray
Copy link
Contributor

@oliverschuerch oliverschuerch added this to the Storybook migration milestone Jan 17, 2024
@oliverschuerch oliverschuerch added research 📦 documentation Related to the @swisspost/design-system-documentation package labels Jan 17, 2024
@imagoiq
Copy link
Contributor Author

imagoiq commented Jan 23, 2024

There is as well a possibility to set an id attribute: https://storybook.js.org/docs/configure/sidebar-and-urls#permalink-to-stories

@gfellerph
Copy link
Member

@imagoiq, is this really a blocker for the storybook migration? If not, please remove the milestone (the ticket will just be with the others in the backlog) so we have a better overview of when we can migrate.

@imagoiq imagoiq removed this from the Storybook migration milestone Jan 31, 2024
@imagoiq imagoiq self-assigned this Feb 5, 2024
@imagoiq imagoiq changed the title Checks links rot with CI Checks links rot with CI + have permalink Feb 5, 2024
@imagoiq imagoiq linked a pull request Feb 5, 2024 that will close this issue
@imagoiq
Copy link
Contributor Author

imagoiq commented Feb 5, 2024

https://storybook.js.org/addons/storybook-redirect has not been updated for 5 years and looks rather complicated.

With #2603, we get kind of permalinks. Nothing prevents us from changing those ids, but we are safe if we can keep ourselves from changing them, or at least do it carefully and add a redirect with a middleware as in storybookjs/storybook#23093 (comment)
This PR doesn't prevent issue when a story is deleted, but we can keep the same conclusion and try to remind ourselves to add a manual redirect.

The only remaining part is external URLs. If we omit Figma, ng-boostrap, badge-fury, archive.design-syste.post.ch, MDN, caniuse (which should be rather stable) we have currently:

1 link on card component (picsum.photos to generated examples)
17 links for logos dependencies on home page

Therefore, it might be overkilled to check external links with a solution involving e2e for example.

@imagoiq
Copy link
Contributor Author

imagoiq commented Feb 14, 2024

@gfellerph I added this ticket again on the migration milestone because having those ids will stop the old link from working, so ideally it should be done when the documentation is publicly available (or before).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
📦 documentation Related to the @swisspost/design-system-documentation package research
Projects
4 participants