Skip to content

Commit 52aef3f

Browse files
docs(agents): correct every rule against the codebase
Validated all 11 files rule by rule against the source. Roughly a third of the rules were wrong, unrunnable, or described a state the repo does not hold. Each finding below was verified directly before the fix. Commands that could not work as written: - `turbo` is not on PATH, so all three `turbo run …` rows failed with "command not found". Now `pnpm exec turbo`. - Turbo filters on the package `name`, not the directory: `--filter=components` matches zero packages. The real names are now listed. - `turbo run typecheck --filter=docs` resolves a task whose command is `<NONEXISTENT>` and exits 0 — a false green. `apps/docs` has no typecheck script at all. Same for stylelint on the two packages lacking it. - `pnpm build:tokens` alone leaves styled-system's generated theme CSS stale. - `pnpm build:doc` already chains generate → ai-docs → skills; the three-command row was redundant, and the MCP server needs `pnpm build:mcp`. Rules the codebase contradicts: - "Rebuild after changing a public API" was wrong. The `hopper-source` export condition resolves packages from source, so typecheck, tests, Storybook and the docs site see TS changes with no rebuild. Split from build-after-install, which is real because CSS resolves from `dist/`. - "Declare devDependencies per package, pnpm does not hoist" was backwards. No package declares `stylelint` or `vitest`; they are root-only by design, and the rule contradicted root AGENTS.md. - Component tokens are shared by *family*, not owned per component. Five families have no owning component; `comp-field` is read by 12 modules. - `packages/components/src/html-elements/` holds only doc previews. The native wrappers are in `packages/styled-system/src/html-wrappers/html.ts`, where the `Html` prefix convention makes a native select `HtmlSelect`. - `Global<Name>CssSelector` is not universal — 14 of 116 components ship none. - The `style` spread order is not settled: 26 files one way, 27 the other. It was stated as a hard rule; now it says to match the file being edited. - The `clsx` argument order carries no meaning and varies by helper; 44 files use `composeClassnameRenderProps`, which went unmentioned. Fabricated examples removed. An agent calibrates on these, so a fake one teaches it to look for the wrong shape of mistake: - `var(--hop-comp-tooltip-color)` in `AvatarGroup.module.css` — inherited from the old AGENTS.md and never real; those tokens appear only in Tooltip. - `[HeadingContext, { fontWeight: … }]` — no context object anywhere passes `fontWeight`. - `padding: 8px` — `px` is outside stylelint's `unit-allowed-list`, so it cannot be written. And one hex colour exists across 90 CSS modules. - `DefaultIconListSlot` — no such identifier; default slots use the `slot()` HOC at export. - `ValidationService.ts`, `ButtonProps.ts`, `breakpoints.ts`, `formatDate.ts`, `useHover.ts` — none exist. Every naming example is now a real file, and `Breakpoints.ts` is noted as a PascalCase exception. - `"$value": "4px"` — component tokens contain no numeric literals; the real cases are three `blur(10px)` values in sharegate. PR #1015's guidelines were proposal language, so component-architecture.md now separates confirmed rules from goals for new API surface, naming where the code diverges: 8 components expose no wrapper prop bag, `Alert`'s `on*ButtonClick` and `TextField.onClear` are typed `() => void` and cannot be fixed without a break, and 18 `ReactNode` content props ship today. `Card` is the counterexample for parent-driven appearance, not the compliant case — `Callout` is. versioning.md now attributes Module Federation to consuming apps (this repo has none), describes the real mechanism — class names version-stamped *and* content-hashed via `localIdentName`, token declarations scoped under a `.hop-<version>-<brand>` root class rather than hashed — and notes the two genuinely unscoped outputs. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
1 parent 274d919 commit 52aef3f

11 files changed

Lines changed: 329 additions & 188 deletions

File tree

.claude/rules/component-css.md

Lines changed: 25 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -7,16 +7,24 @@ paths:
77

88
## Hard Rules
99

10-
| Rule | Violation |
11-
| ----------------------------------------------------------------------------------- | --------------------------------------------------------------- |
12-
| Resolve every color, space, radius, typography and shadow value to a token variable | `padding: 8px`, `color: #666` |
13-
| Read a `--hop-comp-*` token only in the component that owns it | `var(--hop-comp-tooltip-color)` inside `AvatarGroup.module.css` |
14-
| Add a missing value to the owning component's own `*.tokens.json` | Aliasing another component's `--hop-comp-*` token |
15-
| Name selectors `hop-<PascalName>`, `__descendent`, `--modifier` | `.btn`, `.button-label`, `.hop-button` |
10+
| Rule | Violation |
11+
| ---- | --------- |
12+
| Resolve a local declaration to a token, not a raw length | `--hop-MenuItem-sm-padding-block: 0.625rem` instead of a `--hop-space-*` token |
13+
| Read `--hop-comp-<family>-*` only from a module in that family | `list-box/src/ListBoxItem.module.css` reading `--hop-comp-select-*` |
14+
| Locate a token file by grepping its `comp-` key, not by component name | Expecting `checkbox.tokens.json`; the file is `mark.checkbox.tokens.json` |
15+
| Declare a local on the module root when the component has no token file | Adding a 22nd token file for a one-off value |
1616

17-
## The three token layers
17+
Stylelint already owns the mechanical layer: `px` is outside `unit-allowed-list`, and
18+
`selector-class-pattern` enforces `hop-<PascalName>__descendent--modifier`. Both fail at lint, so the
19+
live decisions are the ones above.
1820

19-
A module declares local variables on its root selector and aliases design tokens into them:
21+
## Token families are shared, not per-component
22+
23+
Only 21 token files exist per brand against 90 CSS modules, and five families have no single owning
24+
component — `field` (read by 12 modules), `mark`, `control`, `select`, `tabs`. `comp-button` is read
25+
by 5 modules. Cross-family reads are the design, so the boundary is the *family*, not the file.
26+
27+
## The three layers
2028

2129
```css
2230
.hop-Button {
@@ -25,8 +33,12 @@ A module declares local variables on its root selector and aliases design tokens
2533
}
2634
```
2735

28-
| Layer | Defined in | Usable from |
29-
| --------------------------- | ------------------------------------------------------------- | --------------------------------------- |
30-
| `--hop-comp-<component>-*` | `packages/tokens/src/tokens/components/<brand>/*.tokens.json` | The owning component only |
31-
| `--hop-<category>-*` | `packages/tokens/src/tokens/core/`, `.../semantic/<brand>/` | Any component |
32-
| `--hop-<PascalName>-*` | The component's own `.module.css` | That module only — an internal detail |
36+
| Layer | Defined in | Read from |
37+
| ----- | ---------- | --------- |
38+
| `--hop-comp-<family>-*` | `packages/tokens/src/tokens/components/<brand>/*.tokens.json` | Modules in that family |
39+
| `--hop-<category>-*` | `packages/tokens/src/tokens/core/`, `.../semantic/<brand>/<light\|dark>/` | Any module |
40+
| `--hop-<PascalName>-*` | The module's own root selector | That module — unless deliberately published as a theming hook, as `--hop-RichIcon-*` is for `packages/icons` |
41+
42+
Sibling modules inherit wholesale with `composes: hop-Input from "../../inputs/src/Input.module.css"`,
43+
which pulls in the other module's class *and* its locals. Check what a `composes:` target declares
44+
before adding a local that may already exist there.

.claude/rules/component-tsx.md

Lines changed: 17 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -7,15 +7,16 @@ paths:
77

88
## Hard Rules
99

10-
| Rule | Violation |
11-
| ------------------------------------------------------------------------------------------- | ---------------------------------------------------------------- |
12-
| Pass only `className` through a context object; put the visual value in the CSS module | `[HeadingContext, { fontWeight: "heading-xs-medium" }]` |
13-
| Read browser globals inside an effect or a guard, never at render or module scope | `const w = window.innerWidth` in a component body |
14-
| Derive ids from React's `useId` so the server and client markup agree | `id={Math.random()}`, `Date.now()` at render |
15-
| Export each component in its own statement | `export { _ComboBox as ComboBox, ListBoxItem as ComboBoxItem }` |
10+
| Rule | Violation |
11+
| ---- | --------- |
12+
| Pass `className` and semantic props (`size`, `variant`, `color`, `slot`, `isHidden`) through a context object; leave raw styled-system CSS props out | `[HeadingContext, { fontWeight: "…", padding: "…" }]` |
13+
| Read browser globals inside an effect, a memo, or behind `useIsSSR()` | `window.matchMedia(…)` in a component body, as `SegmentedControlItem.tsx:63` still does |
14+
| Take `useId` from `react-aria` | `import { useId } from "react"` |
15+
| Export each component in its own statement | `export { _ComboBox as ComboBox, ListBoxItem as ComboBoxItem }` |
1616

17-
Grouped exports make `react-docgen-typescript` attribute one component's props to another, which
18-
corrupts the generated documentation. Assign first, then export:
17+
Grouped exports make `react-docgen-typescript` attribute one component's props to another, corrupting
18+
the generated documentation — see `contributing/components.md`. All 101 exports are currently clean;
19+
the rule is a regression guard. Assign first, then export:
1920

2021
```tsx
2122
export const ComboBoxItem = ListBoxItem;
@@ -24,16 +25,21 @@ export { _ComboBox as ComboBox };
2425

2526
## Server rendering
2627

27-
Every component carries a `tests/vitest/<Name>.ssr.test.tsx` proving it renders under Node. Add one
28-
with each new component:
28+
71 of 116 components have a `tests/vitest/<Name>.ssr.test.tsx`. Add one with every new component:
2929

3030
```tsx
3131
/**
3232
* @vitest-environment node
3333
*/
34+
import { renderToString } from "react-dom/server";
35+
36+
import { Button } from "../../src/Button.tsx";
37+
3438
describe("Button", () => {
3539
it("should render on the server", () => {
36-
expect(() => renderToString(<Button>Cutoff</Button>)).not.toThrow();
40+
const renderOnServer = () => renderToString(<Button>Cutoff</Button>);
41+
42+
expect(renderOnServer).not.toThrow();
3743
});
3844
});
3945
```

.claude/rules/package-json.md

Lines changed: 29 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -5,15 +5,35 @@ paths:
55

66
# Package Dependencies
77

8-
Hopper ships inside our products' bundles, so every runtime dependency is downloaded by every end
9-
user — including those on low-bandwidth connections.
8+
These five packages are published and ship inside our products' bundles, so every runtime dependency
9+
is downloaded by every end user — including those on low-bandwidth connections.
1010

1111
## Hard Rules
1212

13-
| Rule | Violation |
14-
| ------------------------------------------------------------------------------------------------- | ---------------------------------------------------- |
15-
| Reach for React, react-aria, TypeScript or CSS before adding a runtime dependency | Adding a date library for one format call |
16-
| Write a small local utility when the need is narrow | Depending on a utility library for one function |
17-
| List a package under `dependencies` only when consumers load it at runtime | A build-only plugin in `dependencies` |
18-
| Declare `devDependencies` in the package that uses them — pnpm workspace does not hoist | Adding a package's dev tooling to the root manifest |
19-
| Keep a shared dependency on one version across the workspace; `pnpm syncpack` is the check | Two packages pinned to different react-aria versions |
13+
| Rule | Violation |
14+
| ---- | --------- |
15+
| Reach for React, react-aria, TypeScript or CSS before adding a runtime dependency | Adding `dayjs` or `date-fns` for one format call |
16+
| Write a small local utility when the need is narrow | Depending on a utility library for one function |
17+
| List a package under `dependencies` only when consumers load it at runtime, or when its types are part of the published API | A build-only plugin in `dependencies` |
18+
| Declare a package's own build and type tooling in that package | Putting `rslib` or `@types/react` only at the root |
19+
| Leave the repo-wide runners at the root and invoke them from there | Adding `vitest` or `stylelint` to a package |
20+
| Keep a shared dependency on one version across the workspace; `pnpm syncpack` is the check | Two packages pinned to different react-aria versions |
21+
22+
## What is already allowed
23+
24+
The react-aria ecosystem is the existing stack, not a new dependency — `@internationalized/date`,
25+
`@react-aria/*`, `@react-stately/*` and `@react-types/shared` are all in scope. Two more are
26+
grandfathered and need no justification:
27+
28+
| Package | Why it stays |
29+
| -------- | ------------------------------------------------------------------------- |
30+
| `clsx` | Used at 78 sites across components, icons and styled-system |
31+
| `csstype`| Types-only, but they are part of styled-system's published API surface |
32+
33+
`svg-icons` and `tokens` have zero runtime dependencies. Keep it that way.
34+
35+
## What syncpack enforces
36+
37+
Beyond a single version repo-wide, `.syncpackrc.js` also enforces range *style*: `^` for published
38+
prod and peer ranges, pinned everywhere else. Two groups are deliberately exempt — `@hopper-ui/*`
39+
prod and peer ranges, and the `react` / `react-dom` peer ranges. Leave both alone.

0 commit comments

Comments
 (0)