swisspost / design-system

The Swiss Post Design System pattern library for a consistent and accessible user experience across the web platform.
https://design-system.post.ch
Apache License 2.0
106 stars 13 forks source link

Checks links rot with CI + have permalink #2465

Closed imagoiq closed 3 months ago

imagoiq commented 4 months ago

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 commented 4 months ago

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…

oliverschuerch commented 4 months ago

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

alizedebray commented 4 months ago

For example: https://storybook.js.org/addons/storybook-redirect/

imagoiq commented 3 months ago

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

gfellerph commented 3 months ago

@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 commented 3 months ago

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

With https://github.com/swisspost/design-system/pull/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 https://github.com/storybookjs/storybook/discussions/23093#discussioncomment-7063363 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 commented 3 months ago

@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).