|
1 | 1 | # Contributing |
2 | 2 |
|
3 | | -Thanks for looking. This is an installed npm component library, published to the npm |
4 | | -registry and developed with [bun](https://bun.sh) as its package manager and task runner. |
5 | | -Read [ARCHITECTURE.md](./ARCHITECTURE.md) first — most of the rules below only make sense |
6 | | -once that is clear. |
7 | | - |
8 | | -## How it works |
9 | | - |
10 | | -Components never fetch; data arrives as props. The root entry is hand-written re-exports |
11 | | -only, and the package is side-effect free apart from its CSS, so either import style |
12 | | -tree-shakes. Tokens and keyframes live in `theme.css`; `styles.css` is the prebuilt sheet |
13 | | -for hosts that do not run Tailwind. [PRODUCT.md](./PRODUCT.md) covers why the library |
14 | | -exists and [IMPLEMENTATION.md](./IMPLEMENTATION.md) the toolchain and peers. |
15 | | - |
16 | | -## Getting set up |
| 3 | +## Development |
17 | 4 |
|
18 | 5 | ```sh |
19 | 6 | bun install |
20 | | -bun run build |
21 | | -``` |
22 | | - |
23 | | -There is no database and no backend. If a change needs one, it belongs in the |
24 | | -consumer's data layer, not in a component — data arrives as props. |
25 | | - |
26 | | -## Gates |
27 | | - |
28 | | -```sh |
29 | | -bun run build # tsc (JS + types) → Tailwind |
30 | | -bun run typecheck # tsc --noEmit |
31 | | -bun run lint # oxlint |
32 | | -bun run format:check # oxfmt --check (`bun run format` rewrites) |
33 | | -bun run dep-guard # forbidden-import checks over src/ |
34 | | -``` |
35 | | - |
36 | | -All five must be clean before a change lands. There is no CI in this repository yet, so |
37 | | -running them locally is the only thing standing between a change and a release. |
38 | | -`typecheck` is not negotiable and `any` is not a way past it: the escapes that exist each |
39 | | -carry a comment explaining why the type system leaves no alternative, and a new one needs |
40 | | -the same. |
41 | | - |
42 | | -## Touching the theme |
43 | | - |
44 | | -The contrast gate is described in [ARCHITECTURE.md](./ARCHITECTURE.md#contrast-is-gated). |
45 | | -A failing pair prints as `FAIL <ratio>:1 (>=<floor>) <mode> <pair>` and exits non-zero. |
46 | | -Two things follow: |
47 | | - |
48 | | -- **Change tokens in `src/theme.css`, then run `bun run build`.** Editing `dist/` changes |
49 | | - nothing — it is generated output and is gitignored. |
50 | | -- **Do not add hex pairs to the test.** The pairs are derived from token _names_. If a |
51 | | - token you added is not covered, the fix is a naming rule in `checksFor`, not a |
52 | | - hardcoded pair. |
53 | | - |
54 | | -If you re-step a `--chart-*` token, re-run the palette validator for the lightness band, |
55 | | -chroma floor and colour-vision separation; those cannot be re-derived from hex pairs, and |
56 | | -the build gate does not cover them. Record the result in the change. |
57 | | - |
58 | | -## The dep-guard |
59 | | - |
60 | | -`scripts/dep-guard.mjs` walks every `.ts` / `.tsx` under `src/` and enforces two rules. |
61 | | -The forbidden strings are named here deliberately — a guard whose forbidden string is a |
62 | | -secret is a guard nobody can comply with. |
63 | | - |
64 | | -1. **No `@workbench/*`.** A published file that imports an unpublished scope is |
65 | | - uninstallable for everyone outside the project that defines it, and the failure lands |
66 | | - in the _consumer's_ install, where they cannot fix it. |
67 | | -2. **Nothing reachable from the root barrel imports an optional peer.** Re-exporting a |
68 | | - module that statically imports an optional peer makes that peer mandatory for every |
69 | | - root import, so such a module must be kept out of the barrel and given its own `exports` entry. |
70 | | - |
71 | | -The same rule, not machine-checked, applies to server-side packages: a component must |
72 | | -never import one. They carry database drivers, and pulling one into a browser bundle is a |
73 | | -category error. Data reaches a component through props. |
74 | | - |
75 | | -Wanting to relax any of this is the signal to add a prop — never to import your way |
76 | | -around it. |
77 | | - |
78 | | -## Adding a component |
79 | | - |
80 | | -1. Write the file under `src/{ui,lib,hooks,blocks}/`. Imports of sibling modules are |
81 | | - **relative and carry a `.js` extension** (`../lib/utils.js`) — there is no path alias. |
82 | | -2. If it is public API, add `export * from "./<dir>/<name>.js";` to `src/index.ts`. |
83 | | - Machinery stays unlisted: keeping something internal is cheap; taking an export back |
84 | | - after it ships is not. |
85 | | -3. If it is public but statically imports an _optional_ peer, leave it out of |
86 | | - `src/index.ts` and add a `./<dir>/<name>` entry to `exports` in `package.json` instead. |
87 | | - `dep-guard` fails if it reaches the barrel. |
88 | | -4. If a `ui/` or `blocks/` module calls a hook, creates a context, defines an inline |
89 | | - event handler, or imports an optional peer, start it with `"use client"`; |
90 | | - `@radix-ui/react-slot` alone does not count. Leave stateless components and |
91 | | - `hooks/` / `lib/` modules unmarked. `dep-guard` fails either way you get it wrong. |
92 | | -5. If it needs a new package, decide deliberately: a framework-level library the consumer |
93 | | - already has is a **peer** dependency, a small implementation detail is a real |
94 | | - dependency. Adding a peer is a breaking change for existing consumers. |
95 | | -6. Run the four gates, then verify a clean install. |
96 | | - |
97 | | -## Verifying a clean install |
98 | | - |
99 | | -A green build proves the package compiles. It does not prove it **installs and renders**, |
100 | | -and the failures that matter most — a dependency you have in this repo but did not |
101 | | -declare, a subpath missing from `exports`, a class the stylesheet does not carry — are |
102 | | -invisible until someone installs into a tree that does not already have them. |
103 | | - |
104 | | -```sh |
105 | | -npm pack --dry-run # inspect the file list |
106 | | -npm pack # → corbits-react-ui-<version>.tgz |
| 7 | +bun run check |
107 | 8 | ``` |
108 | 9 |
|
109 | | -`npm pack` is kept here as a verification command because it is the packer the registry |
110 | | -sees; bun is still the package manager for everything else. Delete the tarball when you |
111 | | -are done — it must not be committed. |
112 | | - |
113 | | -Install that tarball plus the peers into a throwaway consumer app, built from scratch each |
114 | | -time (a directory matching the gitignored `scratch/` pattern is a convenient home for it). |
115 | | -Check two consumers: plain React, and a server-components app using the components both |
116 | | -from a server component and from a `"use client"` file the consumer owns. Build the |
117 | | -consumer from empty every time — one that has accumulated packages from a previous run |
118 | | -will happily compile against a dependency you forgot to declare, which is the exact bug |
119 | | -this catches. |
120 | | - |
121 | | -Two things worth asserting while you are there: |
122 | | - |
123 | | -- **Tree-shaking.** Build one entry that imports a single component and one that imports |
124 | | - everything, and compare. A single component must not drag in the others. |
125 | | -- **The stylesheet.** Check that the classes the rendered markup emits actually have rules |
126 | | - in `dist/styles.css`. Tailwind extracts classes statically, so a utility assembled at |
127 | | - runtime is missing from the sheet and the component is silently unstyled. |
128 | | - |
129 | | -## The component workbench |
130 | | - |
131 | | -`bun run stories` starts [Ladle](https://ladle.dev) — a live, hot-reloading canvas over |
132 | | -every published component, running against `src/theme.css` so what you see is what a |
133 | | -consumer gets. `bun run stories:build` produces a static build of the same canvas for |
134 | | -anyone who wants to check it without running the dev server. |
135 | | - |
136 | | -Stories live under `stories/`, not next to their component in `src/`. SWC and `tsc` |
137 | | -compile all of `src/`, so a colocated `*.stories.tsx` would ship in `dist/`. Keeping them |
138 | | -in a separate top-level directory keeps `dist/` exactly the set of files that matter. |
139 | | - |
140 | | -The theme toggle in Ladle's own top bar switches the real `.dark` class from |
141 | | -`src/theme.css` — not a canvas-only colour swap — so a component's dark-mode tokens, |
142 | | -contrast and all, are what you're actually looking at. |
143 | | - |
144 | | -**A new component's story is part of what "done" means for it**, not a follow-up: at |
145 | | -least one story per meaningful state (loading, empty, error, wherever the component has |
146 | | -one), with fixture data that reads like something a person would actually see — a name, a |
147 | | -title, a sentence — never an id-shaped placeholder. A prop the workbench can't exercise is |
148 | | -a prop nobody has actually looked at rendered. |
149 | | - |
150 | | -## Accessibility |
151 | | - |
152 | | -These components end up in other people's products. Accessibility is a merge requirement, |
153 | | -not a follow-up. |
154 | | - |
155 | | -- **Keyboard.** Every interactive element is reachable and operable from the keyboard |
156 | | - alone — Tab and Shift-Tab in DOM order, Enter/Space to activate, Escape to dismiss an |
157 | | - overlay, arrow keys within a composite widget rather than a Tab stop per option. |
158 | | -- **Focus management.** Focus must always be somewhere sensible and always visible. An |
159 | | - overlay takes focus on open, traps it while modal, and **returns it to the element that |
160 | | - opened it** on close. `src/theme.css` ships one `:focus-visible` ring for the whole app; |
161 | | - primitives do not declare their own and must not suppress it. Never `outline: none` |
162 | | - without a replacement that measures at least 3:1. |
163 | | -- **`aria-*` and semantics.** Prefer a real element over a `div` with a role. Where a role |
164 | | - is needed, wire the states that go with it: `aria-expanded` on a disclosure, |
165 | | - `aria-current` on the active nav item, `aria-live` on a region that updates out of band, |
166 | | - `aria-busy` on a collection **from `isFetching`, not `isLoading`**. Every control has an |
167 | | - accessible name; an icon-only button needs `aria-label`. |
168 | | -- **Colour is never the only channel.** Anything encoded in colour is encoded a second way |
169 | | - too — a shape, an icon, a dash pattern, a label. Charts are the sharp case: the |
170 | | - palette's dark-mode separation is compliant only _because_ every chart ships a legend, |
171 | | - direct labels and a data table. |
172 | | -- Respect `prefers-reduced-motion` for anything that animates. |
| 10 | +`bun run check` runs typecheck, lint, format check and unit tests. `bun run format` rewrites the tree. |
173 | 11 |
|
174 | | -Check your item in both modes and at 200% zoom before opening a change. |
| 12 | +Contributors sign the [CLA](CLA.md) on their first PR; the CLA bot explains how. |
175 | 13 |
|
176 | | -## Naming and scope |
| 14 | +`bun run test:e2e` checks the built package: every `exports` target exists and the |
| 15 | +root entry loads under Node with every optional peer blocked. Run `bun run build` |
| 16 | +first. CI also runs `bun run contrast-test` (theme contrast floors, reads |
| 17 | +`dist/styles.css`) and `bun run dep-guard` (forbidden imports, see `AGENTS.md`) |
| 18 | +after the build. |
177 | 19 |
|
178 | | -- **Generic names only.** Name for the job — agent, workflow, analytics, mail, schedule, |
179 | | - artifact, activity. No product-brand names in a module name, a prop, a type or a |
180 | | - fixture. A name in a published `exports` map is a name we cannot take back without a |
181 | | - major version. |
182 | | -- **The minimum that works.** No wrapper that only forwards props, no config indirection, |
183 | | - no speculative extension point. No CLI, no component generator, no docs site. |
184 | | -- Comments explain _why_. The code already says what. |
| 20 | +## Theme and stories |
185 | 21 |
|
186 | | -## Pull requests |
| 22 | +Change tokens in `src/theme.css`, then run `bun run build`; `dist/` is generated. |
| 23 | +A failing contrast pair prints `FAIL <ratio>:1 (>=<floor>) <mode> <pair>`. Fix it |
| 24 | +with a naming rule in `checksFor`, never a hardcoded hex pair. |
187 | 25 |
|
188 | | -- Keep commits focused, and keep the diff to the change you are describing. |
189 | | -- Explain _why_ in the commit message; the code already says what. |
190 | | -- Run the gates locally and say so in the pull request: `dep-guard`, `typecheck`, `lint`, |
191 | | - and `build` including the contrast gate. Nothing runs them for you yet. |
192 | | -- Contributions are accepted under the repository's LGPL-2.1-only licence. |
| 26 | +`bun run stories` starts the Ladle workbench over every component against |
| 27 | +`src/theme.css`; `bun run stories:build` writes a static copy to `build/`. A new |
| 28 | +component ships with a story for each meaningful state (loading, empty, error). |
193 | 29 |
|
194 | 30 | ## Commit messages |
195 | 31 |
|
|
0 commit comments