docs(ui-react): add Storybook site for shared UI components - #1992
Conversation
Stand up a Storybook site documenting every barrel-exported visual component of @giantswarm/backstage-plugin-ui-react, so humans and agents can tell whether a shared component already exists and how to use it. - Root-level .storybook/ config (Storybook 10 + React/Vite), glob- discovers plugins/ui-react/**/*.stories.tsx - Stories for all 22 visual components: autodocs props tables, key variants/states, a what/when-to-use blurb, and a migration-status note (MUI v4 vs bui) - Global decorator supplies the real GS light/dark themes (reusing the app's buildPalette) with a theme toolbar toggle - Intro MDX landing page + README pointer + a Storybook section in the .claude/skills/ui skill so agents read the in-repo stories as the canonical usage reference - Coverage gate: a pure, unit-tested function + CLI wrapper (yarn storybook:coverage) that fails when an exported component has no story, with an allowlist for intentional exceptions - GitHub Actions workflow: build + coverage gate on PRs, GitHub Pages publish on merge to main No runtime/behavioural changes to any component; the published package output (dist) is unchanged. Part of giantswarm/giantswarm#37264. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The repo-wide `tsc --noEmit` (run without an incremental cache) flagged type errors in five stories that a stale local cache had masked: - Stories that render via `render` with no `args` need the component's required props satisfied at the meta level — add default meta `args` for AsyncValue, Autocomplete, PageHeaderActions, and FiltersLayout. - StackedBarChart is generic; `Meta<typeof StackedBarChart>` collapsed `T` to `object` (making `xAxisKey` resolve to `never`). Type the meta and stories by the concrete `StackedBarChartProps<Row>` instead. - Export the `AsyncValueProps` and `IDateProps` interfaces so the exported story `meta` can name them in its emitted declaration (fixes TS4023). Additive, non-behavioural; also makes the prop types importable by consumers. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
| // theme users see. `buildPalette` is a pure function; with an empty config it | ||
| // returns the built-in GS light/dark palettes (no branding overrides), matching | ||
| // a default deployment. | ||
| import { buildPalette } from '../packages/app/src/modules/app/customThemes'; |
There was a problem hiding this comment.
The Storybook config lives outside tsconfig.json's include (packages/*/src, plugins/*/src only), so yarn tsc/ci:verify never type-checks .storybook/* or scripts/*. Type errors here (this deep app-internal import, the as any on the errorApi stub, the context.globals access in preview.tsx, and everything in scripts/check-story-coverage.mts) are only surfaced by a full storybook:build, and the coverage script is run via tsx (type-stripped, no check) so its type errors are never caught at all. Consider adding these paths to a tsconfig include or a dedicated typecheck step.
There was a problem hiding this comment.
Fixed in 590bdc6. Added tsconfig.storybook.json (includes .storybook/** and scripts/**, noEmit) plus a yarn typecheck:storybook script, wired into this workflow as a step before the build. It immediately paid off — it flagged the untyped bui *.css side-effect import (now covered by a scoped ambient declaration) and the errorApi stub, which is now new MockErrorApi() instead of an as any cast.
| const full = join(dir, entry.name); | ||
| if (entry.isDirectory()) { | ||
| walk(full); | ||
| } else if (entry.name.endsWith('.stories.tsx')) { |
There was a problem hiding this comment.
Coverage gate only counts *.stories.tsx, but .storybook/main.ts discovers stories via *.stories.@(ts|tsx). A story authored as .stories.ts would render in Storybook yet the gate would still report its component as undocumented and fail CI. Match the two globs (accept .stories.ts too).
There was a problem hiding this comment.
Fixed in 590bdc6. The gate now discovers stories with /\.stories\.tsx?$/, matching the *.stories.@(ts|tsx) glob in .storybook/main.ts, so a .stories.ts file counts too.
| const re = /export\s+\*\s+from\s+['"]\.\/(.+?)['"]/g; | ||
| for (const match of source.matchAll(re)) { | ||
| const segments = match[1].split('/'); | ||
| names.add(segments[segments.length - 1]); |
There was a problem hiding this comment.
Exported-component names are derived from the barrel import path segment (export * from './X/Y' -> Y), not the actual exported symbol. It works today only because every directory name equals its component name. If a directory ever exports a differently-named component (or more than one), the gate would demand a story file named after the directory and a correctly-named story wouldn't satisfy it — a confusing false failure.
There was a problem hiding this comment.
Good catch on the implicit assumption. Documented it in 590bdc6: the derivation relies on the repo's one-component-per-directory convention (directory name == component name), and I noted that a differently-named export would require resolving real symbols instead. The new danglingStories check (see the storyCoverage thread) also guards the inverse — a story with no matching export now fails the gate rather than passing silently.
| const storied = new Set(input.storiedComponents); | ||
| const allowlisted = new Set(input.allowlist); | ||
|
|
||
| const undocumented = [...exported] |
There was a problem hiding this comment.
The gate flags undocumented exports and stale allowlist entries, but silently ignores a *.stories.tsx whose component is no longer exported (a dangling story after a rename/removal) and an allowlist entry that is redundantly also storied. Neither is caught, so the story set can drift out of sync with the barrel without failing CI.
There was a problem hiding this comment.
Fixed in 590bdc6. checkStoryCoverage now also returns danglingStories (a *.stories.tsx whose component is no longer exported) and redundantAllowlist (an allowlist entry whose component actually has a story), and the gate fails on both. Added unit tests for each case.
| name: Storybook | ||
|
|
||
| on: | ||
| pull_request: |
There was a problem hiding this comment.
No path filter on the pull_request trigger: every PR (including backend-only or docs-only changes) now pays a full yarn install --immutable + storybook:build. Consider paths: scoping to plugins/ui-react/**, .storybook/**, scripts/check-story-coverage.mts, and this workflow.
There was a problem hiding this comment.
Fixed in 590bdc6. Scoped the pull_request and push triggers with paths: — plugins/ui-react/**, .storybook/**, scripts/check-story-coverage.mts, tsconfig.storybook.json, this workflow, and package.json/yarn.lock (so dependency bumps still trigger it). Left merge_group unscoped so the merge queue always gets a signal.
| // top of each render. | ||
| files: ['src/**/*.stories.tsx'], | ||
| rules: { | ||
| 'react-hooks/rules-of-hooks': 'off', |
There was a problem hiding this comment.
Disabling react-hooks/rules-of-hooks for all story files also masks genuine mistakes (a hook called conditionally / inside a nested callback in a story). The comment asserts hooks are always called unconditionally at the top of render, but nothing now enforces that. A narrower alternative is to keep the rule and structure the interactive stories as small components.
There was a problem hiding this comment.
Agreed — went with the narrower alternative in 590bdc6. The four interactive stories (SingleSelect, MultipleSelect, Autocomplete, YamlEditorFormField) now put their hooks in small named example components, and the blanket react-hooks/rules-of-hooks override is removed, so the rule is back to enforcing across all story files.
- Type-check .storybook/** and scripts/** (they sit outside the root tsconfig include, so ci:verify never checked them, and the coverage script runs via type-stripped tsx). Add tsconfig.storybook.json + a `typecheck:storybook` script, wire it into the workflow, and replace the errorApi `as any` stub with MockErrorApi. - Coverage gate now matches Storybook's story glob (`*.stories.ts` and `*.stories.tsx`), and documents that component names are derived from the barrel path segment (one-component-per-directory invariant). - Harden the coverage gate: also fail on dangling stories (a story whose component is no longer exported) and redundant allowlist entries (an allowlisted component that actually has a story); add unit tests. - Scope the Storybook workflow's pull_request/push triggers with `paths:` so unrelated PRs don't pay for a full install + build. - Refactor interactive stories to small named components instead of calling hooks in a bare `render` callback, and drop the blanket `react-hooks/rules-of-hooks` eslint override. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
JS Dependency Audit0 added · 0 removed · 208 total (0 vs base) Projects audited
No change in vulnerabilities compared to the base branch. Full current vulnerability list (208)
|
What does this PR do?
Stands up a Storybook site documenting Giant Swarm's shared UI component
library,
@giantswarm/backstage-plugin-ui-react, and makes the in-repo storiesthe canonical usage reference. Implements the PRD for epic #37264.
.storybook/(Storybook 10 + React/Vite) thatglob-discovers
plugins/ui-react/**/*.stories.tsx— adding a component with astory needs no config change.
autodocs (auto-generated props table + controls), a default story, the key
variants/states, a short what / when to use it blurb, and a
migration-status note (deprecated MUI v4 vs bui).
(reusing the app's
buildPalette), switchable from a toolbar control — a singleUnifiedThemeProviderthemes MUI v4, MUI v5, and bui at once.ui-reactvs bui vs building new; an agent pointer).
(
plugins/ui-react/src/storybook/storyCoverage.ts) + a thin CLI wrapper(
yarn storybook:coverage) that fails when an exported component has no story,and surfaces stale allowlist entries. Allowlist:
.storybook/story-coverage-allowlist.json(currently empty — 22/22 documented)..github/workflows/storybook.yamlruns the coverage gate + a fullStorybook build on every PR (a broken story fails the pipeline) and publishes
the static site to GitHub Pages on merge to
main.plugins/ui-react/README.md(agent pointer +hooks/utils inventory) and extended the
.claude/skills/uiskill.Source edits to the components are limited to what autodocs needs (no runtime or
behavioural changes); the published package output (
dist) is unchanged.What is the effect of this change to users?
No end-user-facing change. This is a developer/agent-facing documentation and
tooling deliverable: a browsable, themed, always-up-to-date reference for the
shared UI library, published at a stable GitHub Pages URL.
How does it look like?
yarn storybookserves it locally on http://localhost:6006. Each component pagehas a live rendering, interactive controls, an auto-generated props table, its
key states, and a migration note; the Theme toolbar control flips between the
real GS light and dark themes.
Any background context you can provide?
backstage-component-docs/PRD.mdingiantswarm/bumblebee-plansContentRow: #37265 (two divergentContentRowimplementations — theui-reactone is unused and renders itstitle/value flush; consolidation was out of scope here).
Do the docs need to be updated?
This PR is the docs. The
ui-reactREADME and theuiskill are updated in thesame PR.
Should this change be mentioned in the release notes?
No changeset: the change is docs + tooling + dev-dependencies only; the published
@giantswarm/backstage-plugin-ui-reactoutput is unchanged, so there is nothingfor consumers to release.
Ref: https://github.com/giantswarm/giantswarm/issues/37264