refactor(docs): trim inline comments to the traps worth keeping

Author: librowski

apps/docs/scripts/check-ui-component-coverage.mjs

@@ -1,27 +1,8 @@
-// Asserts two things about the documented component surface:
-//
-//   1. every entry in packages/ui/vite.config.mts's `componentEntries` (the
-//      list of publishable subpath bundles) has a matching entry in
-//      `COMPONENTS` (scripts/ui-components.mjs);
-//   2. every `COMPONENTS` entry is actually rendered by an MDX page - a
-//      generated table nobody shows is the same drift in reverse.
-//
-// Without this guard, a new component can be added to the package's public
-// entry points and shipped to npm without ever getting a docs page - nothing
-// else in the build fails, the page just silently never exists. TypeDoc's
-// strict mode doesn't catch this either: it only checks that exported types
-// have doc comments, not that a docs page renders them.
-//
-// A vite entry counts as covered if COMPONENTS has a `dir` equal to the entry
-// name, or nested under it (`${entry}/...`) - entries like `node` bundle
-// several flat docs pages (node-icon, node-description, ...) rather than
-// mapping 1:1 by name. `NARRATIVE_ONLY` is an escape hatch for a vite entry
-// that is deliberately docs-only-by-prose with no generated props table at
-// all (none today - kept for the next one, e.g. a future compound component
-// documented like NodePanel).
-//
-// Wired into apps/docs/package.json's `generate:ui-api` script, right after
-// the generator runs, so `dev` / `build` / `typecheck` all catch the drift.
+// Cross-checks the documented component surface: every publishable subpath in
+// packages/ui/vite.config.mts has a COMPONENTS entry, and every COMPONENTS
+// entry is rendered by an MDX page. Without it a component ships to npm with
+// no docs page and nothing in the build complains. Runs as part of
+// `generate:ui-api`.
 
 import { globSync, readFileSync } from 'node:fs';
 import path from 'node:path';
@@ -37,9 +18,7 @@ const viteConfigPath = path.resolve(repoRoot, 'packages/ui/vite.config.mts');
 
 const NARRATIVE_ONLY = new Set([]);
 
-// The vite entry list is TypeScript, so it is read as text rather than
-// imported. An empty result means the shape changed and every check below
-// would pass over nothing - fail instead.
+// Read as text, not imported - it is TypeScript. Empty means the shape changed.
 function extractComponentEntries(source) {
   const match = /const componentEntries = \[([\s\S]*?)] as const;/.exec(source);
   if (!match) throw new Error('Could not find `componentEntries` in packages/ui/vite.config.mts');
@@ -59,9 +38,6 @@ const missing = componentEntries.filter((entry) => {
   return !componentDirectories.some((directory) => directory === entry || directory.startsWith(`${entry}/`));
 });
 
-// A COMPONENTS entry only produces data - nothing renders it unless an MDX
-// page passes the slug to PropsTable / CssVariablesTable. Without this leg,
-// adding a generator entry with no page passes the whole build silently.
 const contentRoot = path.resolve(documentsRoot, 'src/content/docs');
 const pageSources = globSync('**/*.mdx', { cwd: contentRoot }).map((file) =>
   readFileSync(path.resolve(contentRoot, file), 'utf8'),

apps/docs/scripts/generate-ui-api.mjs

@@ -24,9 +24,7 @@ const uiSource = path.resolve(repoRoot, 'packages/ui/src');
 const outFile = path.resolve(documentsRoot, 'src/generated/ui-api.json');
 const tdJson = path.resolve(documentsRoot, 'node_modules/.cache/ui-typedoc.json');
 
-// Comment patterns that are engineering notes on the token pipeline, not
-// public documentation - stripped so they never render as CSS variable
-// descriptions in the docs (see e.g. status.module.css, icon-size.module.css).
+// Engineering notes in the CSS, never public documentation.
 const INTERNAL_NOTE_RE = /missing token/i;
 
 async function runTypedoc() {
@@ -37,18 +35,10 @@ async function runTypedoc() {
     [
       '--json',
       tdJson,
-      // `expand` over the whole components tree (rather than resolving just
-      // `index.ts`'s re-exports) so that per-variant prop types like
-      // LabelButtonProps/IconButtonProps/IconLabelButtonProps - not
-      // individually re-exported from the package barrel, only reachable
-      // through the Button component's overloaded signature - still get a
-      // full type reflection collectVariantProps can look up by name.
+      // Whole tree, not the barrel: variant prop types are not re-exported.
       '--entryPoints',
       path.resolve(uiSource, 'components'),
-      // `shared` must be an entry too: helper prop types like WithIcon live
-      // there, and a type outside the entry tree gets no reflection - its
-      // intersection members (e.g. Accordion/Modal's `icon`) silently vanish
-      // from the generated tables.
+      // A type outside the entry tree gets no reflection and vanishes from the tables.
       '--entryPoints',
       path.resolve(uiSource, 'shared'),
       '--entryPointStrategy',
@@ -78,7 +68,6 @@ function indexById(root) {
 function findTypeByName(root, name, warnings) {
   const matches = [];
   (function walk(node) {
-    // 2097152 = TypeAlias, 256 = Interface
     if (node.name === name && (node.kind === 2_097_152 || node.kind === 256)) matches.push(node);
     for (const child of node.children ?? []) walk(child);
   })(root);
@@ -169,10 +158,8 @@ function defaultTag(comment) {
   return value || null;
 }
 
-// A component whose props extend a native element's attributes accepts far
-// more than the table lists (`placeholder`, `value`, `onChange`, aria-*, ...).
-// Enumerating ~280 DOM attributes would drown the table, so record which
-// element it forwards to and let the page say so in one line.
+// Which native element a component forwards its remaining props to; listing
+// ~280 DOM attributes in the table would drown the props that are ours.
 const NATIVE_ATTRIBUTE_TYPES = new Map([
   ['InputHTMLAttributes', 'input'],
   ['ButtonHTMLAttributes', 'button'],
@@ -185,34 +172,27 @@ const NATIVE_ATTRIBUTE_TYPES = new Map([
 function findNativeElement(typeNode, byId, depth = 0) {
   if (!typeNode || depth > 8) return null;
   if (typeNode.type === 'reference') {
-    // TypeDoc keeps the qualifier when the import is namespaced (`React.HTMLAttributes`).
     const element = NATIVE_ATTRIBUTE_TYPES.get(typeNode.name.replace(/^React\./, ''));
     if (element === 'element') {
-      // Plain HTMLAttributes<T> - name the element from its type argument.
       const tag = /^HTML(\w*?)Element$/.exec(typeNode.typeArguments?.[0]?.name ?? '')?.[1];
       return tag ? tag.toLowerCase() || 'element' : 'element';
     }
     if (element) return element;
-    // A first-party alias can carry it indirectly (BaseButtonProps -> ButtonHTMLAttributes).
     if (typeof typeNode.target === 'number') {
       const found = findNativeElement(byId.get(typeNode.target)?.type, byId, depth + 1);
       if (found) return found;
     }
   }
-  // The reference is usually wrapped: `Omit<InputHTMLAttributes<…>, 'size'>`.
   for (const nested of [...(typeNode.types ?? []), ...(typeNode.typeArguments ?? [])]) {
     const found = findNativeElement(nested, byId, depth + 1);
     if (found) return found;
   }
   return null;
 }
 
-// Collect own properties from a prop type alias / interface, walking
-// intersections and skipping referenced (extended / native HTML) members.
+// Own properties of a prop type, walking intersections and skipping native members.
 function collectProps(typeNode, byId, accumulator = new Map(), context = null) {
   if (!typeNode) return accumulator;
-  // TypeAlias / Interface: plain object members land directly on `.children`;
-  // computed types (intersections etc.) land on `.type`.
   if (typeNode.kind === 2_097_152 || typeNode.kind === 256) {
     if (typeNode.children?.length) {
       for (const child of typeNode.children) addProperty(child, byId, accumulator);
@@ -230,16 +210,13 @@ function collectProps(typeNode, byId, accumulator = new Map(), context = null) {
   }
   if (typeNode.type === 'reference' && typeof typeNode.target === 'number') {
     const target = byId.get(typeNode.target);
-    // Only follow references into our own package's prop types, not native ones.
-    // Both declaration forms count - a prop type written as an interface is as
-    // valid a target as a type alias.
+    // Follow first-party prop types only; both declaration forms count.
     if (target && (target.kind === 2_097_152 || target.kind === 256)) {
       collectProps(target, byId, accumulator, context);
     }
     return accumulator;
   }
-  // An unfollowed generic reference (Partial<X>, Omit<X, ...>) silently drops
-  // every prop of a first-party X - warn instead of shipping a slimmer table.
+  // Partial<X> / Omit<X, …> would silently drop every prop of X.
   if (typeNode.type === 'reference' && typeNode.typeArguments?.length && context) {
     const firstParty = typeNode.typeArguments.find(
       (argument) => argument.type === 'reference' && typeof argument.target === 'number' && byId.get(argument.target),
@@ -264,12 +241,8 @@ function addProperty(child, byId, accumulator) {
   });
 }
 
-// Merge the prop sets of a discriminated-union component's variants (e.g.
-// Button's Label/Icon/IconLabel components). Props shared by every variant
-// with the same type are documented once, unqualified; props that are
-// variant-specific (missing from, or typed differently in, some variants)
-// get a note appended to their description so the table stays a single flat
-// list without a separate "variants" column.
+// Merges the variants of a union/overload component into one flat table,
+// noting in the description where a prop applies to some variants only.
 function collectVariantProps(propsTypeNames, project, byId, warnings, slug, context) {
   const perVariant = [];
   for (const typeName of propsTypeNames) {
@@ -290,15 +263,13 @@ function collectVariantProps(propsTypeNames, project, byId, warnings, slug, cont
     const occurrences = perVariant
       .filter((variant) => variant.props.has(propertyName))
       .map((variant) => ({ typeName: variant.typeName, prop: variant.props.get(propertyName) }))
-      // `foo?: never` marks a prop as forbidden in that variant - treat it as absent.
+      // `foo?: never` marks a prop forbidden in that variant.
       .filter((occurrence) => occurrence.prop.type !== 'never');
     if (occurrences.length === 0) continue;
     const distinctTypes = new Set(occurrences.map((o) => o.prop.type));
     const sharedByAll = occurrences.length === perVariant.length && distinctTypes.size === 1;
 
-    // Required only when required in EVERY variant: `value` (controlled) and
-    // `defaultValue` (uncontrolled) marked required side by side would
-    // document a call that cannot exist. The variant note carries the detail.
+    // Required in every variant, else the table documents an impossible call.
     const requiredEverywhere =
       occurrences.length === perVariant.length && occurrences.every((o) => o.prop.required);
     const requiredInItsVariants = !requiredEverywhere && occurrences.every((o) => o.prop.required);
@@ -329,20 +300,16 @@ function collectVariantProps(propsTypeNames, project, byId, warnings, slug, cont
 }
 
 function extractCssVariables(directory, warnings, slug) {
-  // An entry that documents an API rather than a styled component (a hook, say)
-  // carries no directory - its page renders the owning component's variables.
+  // No directory - the entry documents an API, not a styled component.
   if (!directory) return [];
 
   const abs = path.resolve(uiSource, 'components', directory);
   if (!existsSync(abs)) {
-    // globSync on a missing directory returns [] - the page would then claim
-    // the component exposes no CSS variables.
     warnings.push(`"${slug}": component directory ${directory} does not exist`);
     return [];
   }
-  // Subcomponents with their own COMPONENTS entry document their own
-  // variables - without this a parent page repeats them and offers overrides
-  // that do nothing there (Button listing NavButton's, Switch IconSwitch's).
+  // Subcomponents with their own page document their own variables; an
+  // override offered on the parent page would do nothing.
   const nestedPrefixes = COMPONENTS.map((component) => component.dir)
     .filter((nested) => nested?.startsWith(`${directory}/`))
     .map((nested) => `${nested.slice(directory.length + 1)}/`);
@@ -354,8 +321,6 @@ function extractCssVariables(directory, warnings, slug) {
   const variables = [];
   for (const file of files) {
     const css = readFileSync(path.resolve(abs, file), 'utf8');
-    // Match `--ax-public-xxx: value` declarations, capturing the value and an
-    // optional same-line comment.
     const re = /(--ax-public-[\w-]+)\s*:\s*([^;]*?)(?:\/\*\s*(.*?)\s*\*\/)?\s*;/g;
     let m;
     while ((m = re.exec(css))) {
@@ -372,10 +337,8 @@ function extractCssVariables(directory, warnings, slug) {
   return variables;
 }
 
-// Groups the docs table by what the variable resolves to, not by what its name
-// suggests: `edge-stroke-width` is a length and `snackbar-success-border` is a
-// color, and neither reads that way from the name alone. Values usually point
-// at a design token, so the token stylesheets are followed to the literal.
+// Groups by what the value resolves to - the name misleads (`edge-stroke-width`
+// is a length, `snackbar-success-border` is a color), so follow it to the literal.
 const LITERAL_COLOR_RE = /^(#|rgb|hsl|oklch|color-mix|linear-gradient|radial-gradient|transparent\b|currentColor\b)/i;
 
 const tokenValues = readTokenValues();
@@ -442,8 +405,7 @@ async function main() {
   console.log('✔ ui-api.json generated\n  ' + summary.join('\n  '));
 
   if (warnings.length > 0) {
-    // An unresolved props type means a rename/typo silently shipped an empty
-    // "no configurable props" page - fail the build instead of warning.
+    // An unresolved type would silently ship a "no configurable props" page.
     console.error('✗ ' + warnings.join('\n✗ '));
     process.exitCode = 1;
   }

apps/docs/scripts/ui-components.mjs

@@ -6,18 +6,13 @@
  * Imported by both the generator and the coverage guard, so the guard can
  * never disagree with the generator about what is documented.
  */
-// slug -> { name, propsType, dir }. `propsType` is the exported prop type the
-// component accepts (or the list of variant prop types for a component whose
-// public surface is a discriminated union - see collectVariantProps), or null
-// when the parts are described in prose instead. `dir` is the component folder
-// under packages/ui/src/components, or null for an entry that documents an API
-// but owns no stylesheet of its own.
+// `propsType`: the exported prop type, a list of variant types, or null when
+// the parts are described in prose. `dir`: the folder under
+// packages/ui/src/components, or null for an entry that owns no stylesheet.
 export const COMPONENTS = [
   { slug: 'accordion', name: 'Accordion', propsType: 'AccordionProps', dir: 'accordion' },
   { slug: 'avatar', name: 'Avatar', propsType: 'AvatarProps', dir: 'avatar' },
-  // Button has no single public props type - it renders one of three variant
-  // components depending on `children` (label / icon / icon+label). Merge
-  // their prop sets instead of documenting only the shared base.
+  // No single props type - one of three variants depending on `children`.
   {
     slug: 'button',
     name: 'Button',
@@ -51,8 +46,7 @@ export const COMPONENTS = [
   { slug: 'switch', name: 'Switch', propsType: 'BaseSwitchProps', dir: 'switch' },
   { slug: 'text-area', name: 'TextArea', propsType: 'TextAreaProps', dir: 'text-area' },
   { slug: 'tooltip', name: 'Tooltip', propsType: 'TooltipProps', dir: 'tooltip' },
-  // Diagram components (props extracted the same way; NodePanel is a compound
-  // component documented narratively, so it has no flat props entry here).
+  // Diagram components.
   { slug: 'node-icon', name: 'NodeIcon', propsType: 'NodeIconProps', dir: 'node/node-icon' },
   {
     slug: 'node-description',
@@ -67,12 +61,8 @@ export const COMPONENTS = [
     dir: 'node/node-as-port-wrapper',
   },
   { slug: 'edge', name: 'EdgeLabel', propsType: 'EdgeLabelProps', dir: 'edge' },
-  // Documented on the Edge page, which already renders the edge variables -
-  // `dir: null` keeps them there instead of splitting them off to a hook that
-  // has no variables section of its own.
+  // Documented on the Edge page, which already renders the edge variables.
   { slug: 'use-edge-style', name: 'useEdgeStyle', propsType: 'UseEdgeStyleParams', dir: null },
-  // Compound component: the parts and their props are described in prose on the
-  // page (like Separator, there is no single props type to generate from), but
-  // its CSS variables are generated.
+  // Compound component - parts described in prose, variables generated.
   { slug: 'node-panel', name: 'NodePanel', propsType: null, dir: 'node/node-panel' },
 ];

apps/docs/src/components/api/css-variables-table.astro

@@ -1,7 +1,5 @@
 ---
-// Renders a component's CSS custom properties (from the component stylesheets,
-// generated into ui-api.json) as a list grouped into Size / Color, mirroring the
-// Overflow UI docs.
+// CSS custom properties of a UI component, generated from source into ui-api.json.
 import data from '../../generated/ui-api.json';
 
 interface CssVariable {

apps/docs/src/components/api/props-table.astro

@@ -1,7 +1,5 @@
 ---
-// Renders a component's props as a card list (name + optional "required" chip +
-// Type / Default + description), generated from `@workflowbuilder/ui` source via
-// TypeDoc. Mirrors the Overflow UI docs - required props first, no `?` marker.
+// Props of a UI component, generated from source into ui-api.json.
 import data from '../../generated/ui-api.json';
 
 interface PropertyRow {
@@ -27,8 +25,6 @@ if (!entry) {
 const props = [...(entry?.props ?? [])].sort(
   (a, b) => Number(b.required) - Number(a.required) || a.name.localeCompare(b.name),
 );
-// Components built on a native element forward everything else to it; listing
-// ~280 DOM attributes would bury the props that are actually ours.
 const nativeElement = entry?.nativeElement ?? null;
 ---
 

apps/docs/src/components/ui-examples/component-preview.tsx

@@ -5,17 +5,10 @@ import { createPortal } from 'react-dom';
 
 import styles from './component-preview.module.css';
 
-// Render examples inside a shadow root so Starlight's stylesheet RULES cannot
-// reach the components (and the library's cannot leak out) - the same
-// isolation the original Overflow UI docs use. Inherited properties
-// (typography, color) and custom properties still cross the shadow boundary
-// by design: that is how the docs theme (`--ax-*` per data-theme) reaches the
-// examples, and why a preview is close to - not pixel-identical with - a
-// consumer app that inherits different page styles.
-//
-// Inside a shadow root `:root` matches nothing, so retarget the library's
-// root-scoped custom-property defaults to `:host`. The stage has a fixed
-// max width, so oversized examples (Snackbar) shrink instead of clipping.
+// Examples render in a shadow root so Starlight's rules cannot reach them and
+// the library's cannot leak out. Inherited and custom properties still cross
+// the boundary - that is how the docs theme reaches the examples. Inside a
+// shadow root `:root` matches nothing, hence the retarget to `:host`.
 const shadowCss = `${`${globalCss}\n${componentCss}`.replaceAll(':root', ':host')}
 :host > :not(style) { max-width: 100%; }`;