diff --git a/.cursor/agents/reviewer-complexity.md b/.cursor/agents/reviewer-complexity.md new file mode 100644 index 0000000..62d6894 --- /dev/null +++ b/.cursor/agents/reviewer-complexity.md @@ -0,0 +1,82 @@ +--- +name: reviewer-complexity +description: Audits branch-modified files against the COMPLEXITY rule heuristics. Use proactively during code reviews to flag overgrown components, services, and REST actions introduced or worsened by the branch. +--- + +You are a complexity auditor for code reviews. + +## When Invoked + +Apply the heuristics defined in `.cursor/rules/COMPLEXITY.mdc` to files modified by the current branch. Pre-existing complexity on the base branch is **out of scope** — only flag what the branch **introduces or worsens**. + +## Workflow + +### Step 1: Determine Base Branch and Modified Files + +This repository uses `develop` as the integration branch. + +```bash +git diff origin/develop...HEAD --name-only -- "*.ts" "*.tsx" \ + | grep -v "\.spec\." \ + | grep -v "\.e2e\.spec\." \ + | grep -v "^e2e/" +``` + +If no files match, simply state: "No source files modified — complexity check skipped." + +### Step 2: Read the Heuristic Table + +Read `.cursor/rules/COMPLEXITY.mdc` to get the current strict thresholds. Do not hard-code the numbers — always reference the rule, so the audit stays in sync when the rule changes. + +### Step 3: Compute Triggers Per File + +For each modified file, count triggers from the heuristic table. Be conservative: only count signals you can verify from the file content. + +When you cannot mechanically count a trigger (e.g. "distinct responsibilities"), apply judgment and explain in the report. + +### Step 4: Compare Against Base + +Use `git show origin/develop:` to read the base version. A trigger only counts if: + +- It is **new** on the branch (didn't exist on `develop`), OR +- The branch **worsened** an existing trigger (e.g. went from 4 hooks to 6). + +Files that were already complex on `develop` and remain unchanged are **not** flagged. + +### Step 5: Map Triggers to Severity + +Per the rule: + +- 3–4 triggers → 🤔 Medium +- 5–6 triggers → 🔥 High +- 7+ triggers → 💀 Critical + +## Output Format + +### If Findings Exist + +For each flagged file: + +```markdown +**``** — ( triggers) + +Triggers: + +- : (threshold ) +- : (threshold ) + +Suggested refactor: +``` + +Group findings by severity (Critical → High → Medium). + +### If No Findings + +Simply state: "Complexity check passed — no new complexity hotspots introduced." + +## Constraints + +- Do not run any build or test commands. This is a static audit. +- Only flag branch-introduced or branch-worsened triggers. +- Cite specific line numbers when possible. +- Do not suggest stylistic refactors that are unrelated to the complexity finding. diff --git a/.cursor/rules/CODE_STYLE.mdc b/.cursor/rules/CODE_STYLE.mdc index 155527f..61caf5b 100644 --- a/.cursor/rules/CODE_STYLE.mdc +++ b/.cursor/rules/CODE_STYLE.mdc @@ -1,321 +1,192 @@ --- -name: Code Style Guidelines -description: Code formatting, naming conventions, and file organization +name: Code Style +description: Naming, formatting, imports, file/code organization for the json-tools frontend app globs: - '**/*.ts' - '**/*.tsx' alwaysApply: true --- -# Code Style Guidelines +# Code Style -## Formatting Standards +Prettier + ESLint enforced. Run `yarn prettier` and `yarn lint`. Generate code that satisfies `@furystack/eslint-plugin` from the start — don't rely on the linter to catch violations. -### Prettier Configuration +## Naming -This project uses Prettier for code formatting. **Always use the project's Prettier configuration**. +| Kind | Convention | Example | +|---|---|---| +| File | kebab-case `.ts` / `.tsx` | `session.ts`, `theme-switch.tsx` | +| Spec | co-located, `.spec.ts` / `.spec.tsx` | `session.spec.ts` | +| E2E spec | `e2e/.e2e.spec.ts` | `e2e/page.spec.ts` | +| Class / type / component / service token | PascalCase | `MonacoModelProvider`, `ScrollService` | +| Function / variable | camelCase, verb-led | `getCurrentUser`, `validateEmail` | +| Constant | UPPER_SNAKE_CASE | `MAX_RETRY_COUNT` | +| Boolean | `is*` / `has*` / `should*` / `can*` prefix | `isAuthenticated` | +| Event handler (internal) | `handle*` | `handleSubmit` | +| Event prop callback | `on*` | `onSave` | +| Type alias | prefer `type` over `interface`. No `I` prefix, no `Type` suffix | `type User = { ... }` | +| Component props | PascalCase + `Props` suffix | `type LoginFormProps` | +| Shade element name | kebab-case, unique, namespaced | `customElementName: 'shade-login'` | -**Configuration file:** `prettier.config.js` +## Imports -```bash -# Format code -yarn format +Order, blank line between groups: -# Check formatting -yarn format:check -``` - -### ESLint Configuration - -This project uses ESLint for code quality. **Always use the project's ESLint configuration**. - -**Configuration file:** `eslint.config.js` - -```bash -# Lint code -yarn lint -``` +1. Node built-ins +2. External deps +3. `@furystack/*` packages +4. Workspace packages (`common`) +5. Relative imports +6. Type-only imports last in each group -### Automated Formatting +```typescript +import { join } from 'path' -Code is automatically formatted on commit via Husky and lint-staged: +import { defineService } from '@furystack/inject' +import { Shade } from '@furystack/shades' -```json -{ - "lint-staged": { - "*.{ts,tsx}": ["eslint --ext .tsx,.ts --cache --fix", "prettier --write", "git add"] - } -} +import { ScrollService } from '../services/scroll-service.js' +import type { JsonSchemaSelectorProps } from './types.js' ``` -## Naming Conventions - -### Files and Directories +Group same-package imports together. Sort alphabetically when many. -#### Source Files - -- **kebab-case** for all files -- `.ts` for TypeScript, `.tsx` for JSX components -- One main export per file +## Workspace layout ``` -✅ Good: -frontend/src/components/theme-switch/index.tsx -frontend/src/services/session.ts -frontend/src/pages/home.tsx - -❌ Avoid: -frontend/src/components/ThemeSwitch.tsx -frontend/src/services/Session.ts -frontend/src/pages/Home.tsx +json-tools/ +├── frontend/ # Shades app (single workspace) +│ └── src/ +│ ├── components/.tsx # leaf components +│ ├── components//index.tsx + assets +│ ├── pages/.tsx # route targets (compare, validate, home) +│ └── services/.ts # frontend services (defineService) +└── e2e/ # Playwright specs ``` -#### Test Files +Use `.js` import suffix on relative paths (NodeNext ESM). No backend service or `common` workspace. -- **Same name as source** with `.test.ts` suffix -- Co-located with source file when possible +## File structure -``` -✅ Good: -frontend/src/environment-options.ts -frontend/src/environment-options.test.ts +```typescript +// 1. imports +import { defineService } from '@furystack/inject' -❌ Avoid: -test/environment-options.test.ts (not co-located) -frontend/src/environment-options.spec.ts (use .test.ts) -``` +// 2. types +type SessionState = 'initializing' | 'unauthenticated' | 'authenticated' -### Components and Classes +// 3. constants +const DEFAULT_TIMEOUT_MS = 5000 -- **PascalCase** for components, classes, and types -- Descriptive and clear purpose +// 4. helpers +const formatError = (err: unknown): string => + err instanceof Error ? err.message : 'Unknown error' -```typescript -// ✅ Good -export const ThemeSwitch = Shade(...) -export const MonacoEditor = Shade(...) -export type EditorOptions = {} - -// ❌ Avoid -export const themeSwitch = Shade(...) // Wrong case -export const editor = Shade(...) // Too generic +// 5. main export +export const ScrollService = defineService({ + name: 'app/ScrollService', + lifetime: 'singleton', + factory: ({ onDispose }) => { /* ... */ }, +}) ``` -### Functions and Variables +## Service / component structure -- **camelCase** for functions and variables -- **UPPER_SNAKE_CASE** for constants +Component render order: services → host props → refs → observable subscriptions → local state → handlers → JSX. ```typescript -// ✅ Good -export function getEnvironmentOptions() {} -export const isValidJson = true -export const DEFAULT_THEME = 'dark' - -// ❌ Avoid -export function GetEnvironmentOptions() {} -export const IsValidJson = true -``` - -## Project Structure +export const JsonSchemaSelector = Shade({ + customElementName: 'json-schema-selector', + render: ({ props, injector, useObservable, useState }) => { + const monacoModels = injector.get(MonacoModelProvider) + const [schema] = useObservable('schema', monacoModels.activeSchema) + const [draft, setDraft] = useState('draft', '') -### Directory Organization + const handleSave = () => { /* ... */ } + return
{/* ... */}
+ }, +}) ``` -json-tools/ -├── frontend/ # Frontend application -│ └── src/ -│ ├── index.tsx # App entry point -│ ├── components/ # Reusable components -│ │ ├── header.tsx -│ │ ├── body.tsx -│ │ ├── layout.tsx -│ │ ├── theme-switch/ -│ │ │ └── index.tsx -│ │ ├── github-logo/ -│ │ │ ├── index.tsx -│ │ │ ├── gh-dark.png -│ │ │ └── gh-light.png -│ │ └── monaco/ -│ │ ├── monaco-editor.tsx -│ │ └── monaco-diff-editor.tsx -│ ├── pages/ # Page components -│ │ ├── home.tsx -│ │ ├── compare.tsx -│ │ └── validate.tsx -│ ├── services/ # Frontend services -│ └── themes/ # Theme definitions -└── e2e/ # End-to-end tests - └── page.spec.ts -``` - -### Component File Structure -Components should be organized in directories when they have assets: +## JSX -``` -components/ -├── theme-switch/ -│ └── index.tsx # Component with no assets -├── github-logo/ -│ ├── index.tsx # Component -│ ├── gh-dark.png # Dark theme asset -│ └── gh-light.png # Light theme asset -└── header.tsx # Simple component (no directory needed) -``` +Multi-line for >2 attrs. Single-line for trivial cases. No nested ternaries — use early returns for >2 branches. -## Import Ordering +```tsx +// ✅ + -### Import Order +// ❌ nested ternary +{isLoading ? : error ? : } +``` -1. **Node built-ins** -2. **External dependencies** (npm packages) -3. **FuryStack packages** (`@furystack/*`) -4. **Relative imports** -5. **Type imports** (if separated) +## JSDoc -```typescript -// ✅ Good - organized imports -import { EventEmitter } from 'events' +Required on exported services / components / endpoints. For internal symbols, JsDoc is optional — only add it when it passes the value test below. -import { Shade } from '@furystack/shades' -import { Injector } from '@furystack/inject' +### Value test -import { MonacoEditor } from '../components/monaco/monaco-editor.js' -import type { EditorOptions } from './types.js' +Before writing or keeping JsDoc, strip the type signature mentally and read the JsDoc alone. If it still tells the reader something the type does not, keep it. If it just narrates the type, delete it. -// ❌ Avoid - random ordering -import { MonacoEditor } from '../components/monaco/monaco-editor.js' -import { EventEmitter } from 'events' -import { Injector } from '@furystack/inject' -``` +Keep when JsDoc explains: -### Import Grouping +- **Intent** — why this exists, what problem it solves +- **Trade-offs** — design choices that have alternatives +- **Constraints** — runtime invariants, ordering, lifecycle, side effects +- **Non-obvious usage** — patterns the type allows but discourages, or shapes the caller is expected to follow -Separate import groups with blank lines: +Delete when JsDoc: -```typescript -// ✅ Good - separated groups -import { Injector } from '@furystack/inject' -import { Shade } from '@furystack/shades' +- Restates the function name (`isJsonObject` → "Runtime type guard for a JSON object.") +- Restates parameter or return types already in the signature +- Says "Service token for monaco models" when the type is literally `Token` -import { MonacoEditor } from './components/monaco/monaco-editor.js' -``` +### `@example` -## Code Organization +Include when usage is not obvious from the signature. Examples must be copy-paste compilable — if you copy the block into a fresh `.ts` file and add the imports shown, `tsc --noEmit` passes. Drift is the main failure mode; treat examples as code, not prose. -### Component File Structure +Cross-link instead of duplicating. Write `See {@link MonacoModelProvider} for active-model semantics` rather than re-explaining it on every consumer. -```typescript -// 1. Imports -import { Shade } from '@furystack/shades'; - -// 2. Types -type MyComponentProps = { - title: string; -}; - -// 3. Component -export const MyComponent = Shade({ - customElementName: 'my-component', - render: ({ props }) => { - return
{props.title}
; - }, -}); -``` +### Cross-file `{@link}` references -### Service File Structure +When a `{@link Symbol}` references a symbol declared in another file, add a type-only import for that symbol at the top of the referencing file: ```typescript -// 1. Imports -import { Injectable, Injected } from '@furystack/inject' -import { ObservableValue } from '@furystack/utils' - -// 2. Types -type ServiceState = { - data: string -} - -// 3. Service class -@Injectable({ lifetime: 'singleton' }) -export class MyService { - // Injected dependencies - @Injected(OtherService) - declare private otherService: OtherService - - // Observable state - public state = new ObservableValue(null) - - // Public methods - public getData(): string { - return this.state.getValue()?.data ?? '' - } -} +import type { MonacoModelProvider } from '../services/monaco-model-provider.js' ``` -## JSX/TSX Code Style - -### JSX Formatting - -```tsx -// ✅ Good - multi-line for readability - - -// ✅ Good - single line for simple cases - +TypeScript recognizes JsDoc references as usage (no `noUnusedLocals` complaint) and IDE navigation works. Type-only imports are erased under `verbatimModuleSyntax: true`, so circular type-only imports between two files are fine. -// ❌ Avoid - hard to read - -``` +### Example — value test in practice -### Conditional Rendering +```typescript +// ✅ keeps — explains intent + trade-off, irreplaceable by the type +/** + * Schemas are cached by URL; the cache is process-lifetime since validators + * are pure and re-fetching adds latency on every editor change. + */ +const schemaCache = new Map() -```tsx -// ✅ Good - logical AND for simple cases -{ - isLoading &&
Loading...
-} - -// ✅ Good - ternary for if-else -{ - isLoading ?
Loading...
:
Content
-} - -// ✅ Good - early return for complex cases -if (isLoading) return
Loading...
-if (error) return
Error: {error}
-return
Content
- -// ❌ Avoid - nested ternaries -{ - isLoading ?
Loading...
: error ?
Error
:
Content
-} +// ❌ deletes — narrates the type, no extra signal +/** + * Runtime type guard for {@link JsonSchema}. + */ +export const isJsonSchema = (value: unknown): value is JsonSchema => { /* ... */ } ``` -## Summary +## Comments -**Key Principles:** +Only for non-obvious intent / trade-offs / constraints. Never narrate what code does. -1. **Follow project's Prettier and ESLint configs** -2. **Use consistent naming:** - - PascalCase: Components, classes, types - - camelCase: Functions, variables - - kebab-case: File names - - UPPER_SNAKE_CASE: Constants -3. **Organize imports** by category with blank lines -4. **Co-locate tests** with source files -5. **Structure components** in directories when they have assets - -**Tools:** +```typescript +// ✅ +// Exponential backoff: 1s, 2s, 4s, 8s, capped at 30s +const delay = Math.min(1000 * 2 ** attempt, 30000) -- Prettier: `yarn format` -- ESLint: `yarn lint` -- Type Check: `yarn build` -- Unit Tests: `yarn test` -- E2E Tests: `yarn test:e2e` +// ❌ +// Set count to 0 +const count = 0 +``` diff --git a/.cursor/rules/COMPLEXITY.mdc b/.cursor/rules/COMPLEXITY.mdc new file mode 100644 index 0000000..db618c2 --- /dev/null +++ b/.cursor/rules/COMPLEXITY.mdc @@ -0,0 +1,64 @@ +--- +name: Complexity Thresholds +description: Strict heuristic checklist to flag overgrown Shade components and frontend services +globs: + - 'frontend/**/*.ts' + - 'frontend/**/*.tsx' + - '!**/*.spec.ts' + - '!**/*.spec.tsx' + - '!**/*.e2e.spec.ts' + - '!e2e/**' +alwaysApply: true +--- + +# Complexity Thresholds + +Flag whether a unit (Shade component, `defineService` factory, hook helper) has grown too complex and warrants refactoring. + +## When to apply + +- **Code reviews** — check branch-modified `.ts` / `.tsx` files. Pre-existing complexity on the base branch is out of scope; only flag what the branch **introduces or worsens**. +- **During implementation** — proactively decompose when crossing thresholds. + +## Heuristic table + +Flag when **3+ triggers** hit in a single file. Strict thresholds: + +| Signal | Trigger | Applies to | +|---|---|---| +| Multiple concerns | Unit owns >2 distinct responsibilities (state domains, side effects, UI sections) | all | +| Observable sprawl | >3 `useObservable` / `useDisposable` calls in one `render` | Shade `.tsx` | +| Subscription density | >1 manual `.subscribe()` inside a render or factory body | Shade, service | +| Token fan-out | >4 distinct `injector.get(...)` / `inject(...)` calls in one unit | all | +| Service factory deps | `defineService` factory injects >4 tokens | service `.ts` | +| Service factory length | factory body >120 lines | service `.ts` | +| Handler bloat | any handler / callback body >15 lines | Shade | +| Prop count | Shade accepts >6 props (excluding `children`, `className`) | Shade `.tsx` | +| Render length | JSX return block >60 lines | Shade `.tsx` | +| Type sprawl | single exported type definition >30 lines | all | +| Test mock load | spec file requires >8 hoisted mocks | `*.spec.*` | +| File length (soft) | Shade `.tsx` >200 / service `.ts` >150 | per file kind | + +`File length` is a soft signal — flag only when other triggers also hit. + +## Severity + +| Triggers hit | Severity | Required action | +|---|---|---| +| 3–4 | 🤔 Medium | address in this PR or follow-up | +| 5–6 | 🔥 High | refactor before merge | +| 7+ | 💀 Critical | must refactor; complexity is a maintenance risk | + +## Refactoring patterns + +1. **Extract a render-hook helper** — pull a coherent slice of `render` (state + subs + handlers) into a module-level function (`use-.ts`) that takes the destructured render context and returns `{ state, handlers }`. +2. **Split into sub-Shades** — extract presentational regions into their own `Shade()` with focused props. +3. **Extract pure helpers** — pure functions in sibling files (`format-foo.ts`, `validate-bar.ts`). No DI, no observables — easy to unit-test. +4. **Service decomposition** — split a fat `defineService` into multiple cooperating services. + +## Targets after refactoring + +- Orchestrator Shade: ~80–120 lines, wires services and renders sub-components. +- Each extracted helper / sub-Shade: single responsibility, <100 lines. +- Service factory: <120 lines, ≤4 deps. +- Spec files: <250 lines, <8 hoisted mocks. diff --git a/.cursor/rules/SHADES_COMPONENTS.mdc b/.cursor/rules/SHADES_COMPONENTS.mdc new file mode 100644 index 0000000..ecdb530 --- /dev/null +++ b/.cursor/rules/SHADES_COMPONENTS.mdc @@ -0,0 +1,211 @@ +--- +name: Shades Components +description: Frontend Shade component patterns with @furystack/shades and @furystack/shades-common-components +globs: + - 'frontend/**/*.tsx' + - 'frontend/**/*.ts' +alwaysApply: false +--- + +# Shades Components + +`@furystack/eslint-plugin` enforces most of these — generate compliant code from the start. + +## Lint enforces + +| Topic | Rule | +|---|---| +| No module-level JSX | `no-module-level-jsx` | +| No manual `.subscribe()` in render | `no-manual-subscribe-in-render` | +| No `.getValue()` without `useObservable` | `no-direct-get-value-in-render` | +| Prefer `useState` over manual `ObservableValue` | `prefer-use-state` | +| No `useState` for CSS states | `no-css-state-hooks` | +| Valid `customElementName` | `valid-custom-element-name` | +| `LocationService` + `NestedRouteLink` for navigation | `prefer-location-service`, `prefer-nested-route-link` | +| `tabindex` with spatial nav targets | `require-tabindex-with-spatial-nav-target` | + +## Anatomy + +```typescript +import { Shade } from '@furystack/shades' + +type GreetingProps = { + name: string + onGreet?: () => void +} + +export const Greeting = Shade({ + customElementName: 'app-greeting', + css: { + padding: '8px 16px', + '&:hover': { backgroundColor: cssVariableTheme.background.paper }, + }, + render: ({ props, useState }) => { + const [count, setCount] = useState('count', 0) + return ( + + ) + }, +}) +``` + +- `customElementName`: kebab-case, unique, namespaced. +- Props: `type`, PascalCase, `Props` suffix. +- Pull only the render hooks you actually use from the destructured arg. + +## Render hooks + +| Need | Hook | +|---|---| +| Local state | `useState('key', initial)` | +| Subscribe to a service `ObservableValue` | `useObservable('key', observable)` | +| Own a long-lived disposable | `useDisposable('key', () => factoryFn())` | +| Direct DOM access (focus / scroll / animate) | `useRef('key')` | +| Set host attributes / styles | `useHostProps({ ... })` | +| Resolve a service | `injector.get(ServiceToken)` | + +## Removed APIs (forbidden) + +- `element` in render args → use `useHostProps` / `useRef` +- `onAttach` / `onDetach` in `ShadeOptions` → use `useDisposable` + +## DI in render + +Use `injector.get(ServiceToken)` to resolve services. Boilerplate uses **functional DI** — services are `defineService` tokens, not classes. + +```typescript +render: ({ injector }) => { + const monacoModels = injector.get(MonacoModelProvider) + const themeProvider = injector.get(ThemeProviderService) + // ... +} +``` + +`injector.getInstance()` is the legacy decorator-era API and is removed. + +## State management + +### Local state — `useState` over manual `ObservableValue` + +`prefer-use-state` lint rule enforces. Reserve `useDisposable` + `ObservableValue` for state that crosses component boundaries via a service (not just parent → child — for that, pass plain props). + +### Service-owned state — `useObservable` + +```typescript +render: ({ injector, useObservable }) => { + const provider = injector.get(MonacoModelProvider) + const [schema] = useObservable('schema', provider.activeSchema) + const [isLoading] = useObservable('isLoading', provider.isLoading) + if (isLoading) return
Loading...
+ return
{schema?.$id ?? 'No schema'}
+} +``` + +`no-getvalue-without-useobservable` enforces — do not call `.getValue()` in render to read observable state. + +### No manual `.subscribe()` in render + +```typescript +// ❌ enforced by no-manual-subscribe-in-render +themeProvider.subscribe('themeChanged', (t) => setTheme(t)) + +// ✅ wire through useDisposable +useDisposable('theme', () => + themeProvider.subscribe('themeChanged', (t) => setTheme(t)), +) +``` + +## Refs + +Refs are null on first sync render. Defer access via `queueMicrotask`. Don't create refs you never read from. For `classList.add` toggling, prefer `useState` + declarative JSX. + +```typescript +// ❌ ref used only for classList +const ref = useRef('backdrop') +requestAnimationFrame(() => ref.current?.classList.add('visible')) + +// ✅ declarative +const [isVisible, setIsVisible] = useState('isVisible', false) +requestAnimationFrame(() => setIsVisible(true)) +return
...
+``` + +## Styling — `css` over `style` for component styles + +Use the `css` property for defaults + pseudo-selectors. Use `style` only for per-instance dynamic values from props. + +```typescript +const Button = Shade({ + customElementName: 'app-button', + css: { + padding: '12px 24px', + backgroundColor: 'blue', + '&:hover': { backgroundColor: 'darkblue' }, + '&:disabled': { opacity: '0.5', cursor: 'not-allowed' }, + }, + render: ({ props }) => , +}) +``` + +`no-css-state-hooks` forbids `useState` for hover / focus / active — use `&:hover` etc. in `css`. + +### Theme variables — type-safe + +```typescript +import { cssVariableTheme } from '@furystack/shades-common-components' + +css: { + color: cssVariableTheme.text.primary, + backgroundColor: cssVariableTheme.background.paper, + borderColor: cssVariableTheme.palette.primary.main, +} +``` + +### Anti-patterns to avoid + +- Static styles in `useHostProps({ style })` — put them in `css`. +- Attribute-driven styles toggled in JS — write a CSS rule on the data attribute instead. + +## Module-level JSX is forbidden + +`no-module-level-jsx` enforces. JSX returns a VNode; reusing the same instance across mounts breaks reconciliation. Use factory functions. + +## VNode reconciliation is positional + +No keys. List reorders patch/replace from the change point. Wrap each list item in a Shade component to isolate inner-DOM churn. + +```typescript +// ❌ raw
  • reorders churn the DOM +{items.map(item =>
  • {item.name}
  • )} + +// ✅ component boundary +{items.map(item => )} +``` + +## Microtask batching + +State changes within one sync execution coalesce into a single render. Tests must `await flushUpdates()` before asserting DOM state. + +## Common components + +`@furystack/shades-common-components` ships building blocks: `AppBar`, `Button`, `Form`, `Input`, `Paper`, `Typography`, `DrawerToggleButton`, `NestedRouteLink`, `NotyService`, `ThemeProviderService`, etc. Prefer them over hand-rolled equivalents — they handle theming, accessibility, and composition correctly. + +## Routing + +Use `NestedRouteLink` and `LocationService` for navigation, not direct `window.location` or hash manipulation. The `prefer-location-service` and `prefer-nested-route-link` lint rules enforce this. + +## App entry point + +```typescript +import { createInjector } from '@furystack/inject' +import { initializeShadeRoot } from '@furystack/shades' +import { ThemeProviderService, defaultDarkTheme } from '@furystack/shades-common-components' + +const injector = createInjector() +injector.get(ThemeProviderService).setAssignedTheme(defaultDarkTheme) + +const rootElement = document.getElementById('root') as HTMLDivElement +initializeShadeRoot({ injector, rootElement, jsxElement: }) +``` diff --git a/.cursor/rules/TESTING_GUIDELINES.mdc b/.cursor/rules/TESTING_GUIDELINES.mdc index 3eb7ab7..b8a5c58 100644 --- a/.cursor/rules/TESTING_GUIDELINES.mdc +++ b/.cursor/rules/TESTING_GUIDELINES.mdc @@ -1,6 +1,6 @@ --- name: Testing Guidelines -description: Unit testing with Vitest and E2E testing with Playwright +description: Vitest unit tests + Playwright E2E for the json-tools frontend app globs: - '**/*.spec.ts' - '**/*.spec.tsx' @@ -10,225 +10,119 @@ alwaysApply: false # Testing Guidelines -## Unit Testing with Vitest +Vitest for unit + integration. Playwright for E2E. Specs co-located with source as `*.spec.ts(x)`. E2E in `e2e/.spec.ts` (or `e2e/.e2e.spec.ts`). -### Test Structure +## Workspace -```typescript -// ✅ Good - clear test structure -describe('EnvironmentOptions', () => { - describe('getEnvironmentOptions', () => { - it('should return default options when no URL params', () => { - // Arrange - const url = new URL('http://localhost') - - // Act - const result = getEnvironmentOptions(url) - - // Assert - expect(result.theme).toBe('dark') - }) - }) -}) -``` +Single `frontend` workspace. `vitest.config.mts` runs jsdom tests against `frontend/src/**/*.spec.(ts|tsx)`. Run all with `yarn test`. -### Test All Exported Functions +## Disposable safety — wrap everything in `using` / `usingAsync` -Every exported function should have tests: +All disposable resources in tests **must** be wrapped. Manual `[Symbol.dispose]()` at the end leaks if the test throws first. ```typescript -// ✅ Good - testing public API -describe('orderFields', () => { - it('should order object fields alphabetically', () => { - const input = { z: 1, a: 2, m: 3 } - const result = orderFields(input) - expect(Object.keys(result)).toEqual(['a', 'm', 'z']) - }) - - it('should handle nested objects', () => { - const input = { outer: { z: 1, a: 2 } } - const result = orderFields(input) - expect(Object.keys(result.outer)).toEqual(['a', 'z']) - }) - - it('should handle empty objects', () => { - const result = orderFields({}) - expect(result).toEqual({}) - }) -}) -``` - -### Test Edge Cases +import { using, usingAsync } from '@furystack/utils' +import { createInjector } from '@furystack/inject' -Test boundary conditions and error scenarios: - -```typescript -// ✅ Good - testing edge cases -describe('parseJson', () => { - it('should handle valid JSON', () => { - const result = parseJson('{"key": "value"}') - expect(result).toEqual({ key: 'value' }) - }) - - it('should handle invalid JSON', () => { - const result = parseJson('invalid') - expect(result).toBeNull() - }) - - it('should handle empty string', () => { - const result = parseJson('') - expect(result).toBeNull() - }) - - it('should handle null input', () => { - const result = parseJson(null as unknown as string) - expect(result).toBeNull() - }) +// ✅ +await usingAsync(createInjector(), async (injector) => { + const provider = injector.get(MonacoModelProvider) + expect(provider.activeSchema.getValue()).toBeNull() }) + +// ❌ leaks on throw +const injector = createInjector() +const provider = injector.get(MonacoModelProvider) +expect(...).toBe(...) +await injector[Symbol.asyncDispose]() ``` -### Avoid False Positive Tests +Common disposables: `Injector`, `ObservableValue`, `Cache`, stores, services that own them. -Ensure assertions always run: +**Exception:** when the test verifies post-dispose behavior itself, manual disposal is required. -```typescript -// ❌ False positive risk - passes if function doesn't throw -it('should throw on error', async () => { - try { - await functionThatShouldThrow() - } catch (error) { - expect(error).toBeInstanceOf(Error) - } -}) +## Mocking — minimal, via `injector.bind` -// ✅ Correct - fails if assertions don't run -it('should throw on error', async () => { - expect.assertions(1) - try { - await functionThatShouldThrow() - } catch (error) { - expect(error).toBeInstanceOf(Error) - } -}) +Substitute services with the same mechanism production uses for bootstrap overrides. Avoid `vi.fn()` mocks for FuryStack services. -// ✅ Better - use expect().rejects -it('should throw on error', async () => { - await expect(functionThatShouldThrow()).rejects.toThrow() +```typescript +await usingAsync(createInjector(), async (injector) => { + injector.bind(MonacoModelProvider, () => ({ + activeSchema: new ObservableValue(null), + setActiveSchema: vi.fn(), + })) + const consumer = injector.get(SchemaSelectorService) + // ... }) ``` -## E2E Testing with Playwright - -### Page Object Pattern - -```typescript -// e2e/page.spec.ts -import { test, expect } from '@playwright/test' - -test.describe('Home Page', () => { - test.beforeEach(async ({ page }) => { - await page.goto('/') - }) +Use `vi.fn()` only for non-DI externals (e.g. raw `fetch` callbacks). - test('should display editor', async ({ page }) => { - await expect(page.locator('.monaco-editor')).toBeVisible() - }) +## When to test null / undefined / "wrong type" inputs - test('should format JSON on button click', async ({ page }) => { - const editor = page.locator('.monaco-editor') - await editor.fill('{"z":1,"a":2}') +**Rationale:** older guidance asked for blanket null/undefined/error tests on every export. That asserts TypeScript works (which it does) and encourages defensive guards for impossible-by-types inputs. Trust the TS type system; consumers will use it too. - await page.click('button:has-text("Format")') +You **MUST** test null / undefined / malformed values when: - // Verify formatted output - await expect(editor).toContainText('"a": 2') - }) -}) -``` +- **External boundary, no schema validation upstream** — REST handlers not wrapped in `Validate(...)`, WebSocket message handlers, file parsers, env-var consumers, anything reading `unknown` or `JSON.parse` output. At runtime nothing prevents a bad payload, so the function is the line of defense. +- **The type signature itself allows the case** — `T | null`, `T | undefined`, `unknown`, `T?`. The function declared the case as legal, so test the documented behavior on each branch. -### Visual Regression Testing +If a `Validate(...)` (or JSON-schema / zod) wrapper sits ahead of a REST handler, the wrapper covers bad-payload tests — the inner handler should not retest them. -```typescript -// ✅ Good - visual snapshot testing -test('should match visual snapshot', async ({ page }) => { - await page.goto('/') - await expect(page).toHaveScreenshot('home-content.png') -}) +Outside those cases, do **not** add such tests. -test('should match title snapshot', async ({ page }) => { - await page.goto('/') - const title = page.locator('h1') - await expect(title).toHaveScreenshot('title.png') -}) -``` +## Shade component tests -### Waiting for Elements +Frontend project uses `environment: 'jsdom'`. Renders are batched via `queueMicrotask` — await `flushUpdates()` before asserting DOM state. ```typescript -// ✅ Good - wait for element to be ready -test('should load editor', async ({ page }) => { - await page.goto('/') - - // Wait for Monaco editor to initialize - await page.waitForSelector('.monaco-editor', { state: 'visible' }) +import { flushUpdates } from '@furystack/shades' - // Now interact with the editor - await page.click('.monaco-editor') +it('updates after state change', async () => { + // trigger state change + await flushUpdates() + expect(element.textContent).toBe('updated') }) ``` -## Test Coverage +If a render triggers cascaded updates, await `flushUpdates()` again. -### Coverage Requirements +## E2E tests (Playwright) -- **Exported functions**: 100% coverage required -- **Components**: Key render paths covered -- **E2E**: Critical user flows covered +Locate Shade components by their `customElementName`. Use semantic locators inside (`getByRole`, `getByLabel`, `getByText`). -```bash -# Run unit tests with coverage -yarn test --coverage - -# Run E2E tests -yarn test:e2e -``` - -## Running Tests - -### Available Commands +```typescript +import { expect, test } from '@playwright/test' -```bash -# Unit tests -yarn test +test('validates JSON against a schema', async ({ page }) => { + await page.goto('/validate') + await expect(page.locator('json-schema-selector')).toBeVisible() -# E2E tests -yarn test:e2e + await page.getByRole('combobox', { name: 'Schema' }).selectOption('person') + await page.locator('monaco-editor').click() + await page.keyboard.type('{"name":"Ada","age":30}') -# All tests -yarn test && yarn test:e2e + await expect(page.locator('[data-testid="validation-result"]')).toContainText('Valid') +}) ``` -## Summary - -**Key Principles:** - -1. **Test all exported APIs** - 100% coverage for exports -2. **Test edge cases** - Null, undefined, errors, empty states -3. **Avoid false positives** - Use `expect.assertions()` when needed -4. **E2E for critical flows** - Test main user journeys -5. **Visual regression** - Snapshot testing for UI -6. **Clear test structure** - describe > describe > it +Prefer Playwright's auto-wait (`expect(...).toBeVisible()`) over `waitForSelector`. -**Testing Checklist:** +## False-positive guards -- [ ] All exported functions have tests -- [ ] Edge cases tested (null, undefined, errors) -- [ ] Error paths tested -- [ ] E2E tests cover critical user flows -- [ ] Visual snapshots updated when UI changes +- Assertions inside `try/catch` → use `expect.assertions(N)` so the test fails if the function unexpectedly resolves. +- Assertions inside `.forEach` / `.map` callbacks → same risk. +- Async assertions without `await` → promise resolves after the test completes. +- Mocks that always succeed → make at least one test exercise failure. -**Tools:** +## Commands -- Unit Tests: `vitest` -- E2E Tests: `playwright` -- Commands: `yarn test`, `yarn test:e2e` +| Purpose | Command | +|---|---| +| All unit tests | `yarn test` | +| Coverage | `yarn test --coverage` | +| Watch | `yarn test --watch` | +| All E2E | `yarn test:e2e` | +| One E2E spec | `yarn playwright test e2e/` | +| Headed E2E | `yarn playwright test --headed` | +| Debug E2E | `yarn playwright test --debug` | diff --git a/.cursor/rules/TYPESCRIPT_GUIDELINES.mdc b/.cursor/rules/TYPESCRIPT_GUIDELINES.mdc index a1780da..dbb81bd 100644 --- a/.cursor/rules/TYPESCRIPT_GUIDELINES.mdc +++ b/.cursor/rules/TYPESCRIPT_GUIDELINES.mdc @@ -1,6 +1,6 @@ --- name: TypeScript Guidelines -description: TypeScript type safety and strict typing guidelines +description: Type safety, strict typing, no `any`, generics, type guards for the json-tools frontend app globs: - '**/*.ts' - '**/*.tsx' @@ -9,258 +9,139 @@ alwaysApply: true # TypeScript Guidelines -## Type Safety +## Public API -### NEVER use `any` - -There are no acceptable uses of `any` in this codebase: +- All exports have explicit types (params + return). +- Generic params have constraints + descriptive names (`TUser`, `TResult` — not `T`, `R`). +- Prefer `type` over `interface`. No `I` prefix, no `Type` suffix. ```typescript -// ✅ Good - use unknown for truly unknown types -export function processData(data: unknown): string { - if (typeof data === 'string') { - return data; - } - return JSON.stringify(data); -} - -// ✅ Good - use generics for flexible types -export function identity(value: T): T { - return value; -} - -// ❌ FORBIDDEN - using any -export function processData(data: any): string { - return data.toString(); -} +// ✅ +export const MonacoModelProvider = defineService({ + name: 'app/MonacoModelProvider', + lifetime: 'singleton', + factory: (): MonacoModelProvider => ({ /* ... */ }), +}) + +// ❌ implicit return type, single-letter generic +export function fetchData(url) { return fetch(url) } ``` -### Explicit Types for Public APIs +## NO `any` — one exception -All exported functions and public methods should have explicit types: +`any` forbidden in values, params, return types. **Exception:** generic constraint position when variance breaks `unknown` inference (e.g. callback param positions). ```typescript -// ✅ Good - explicit types -export function getEnvironmentOptions(): EnvironmentOptions { - // Implementation -} - -export class EditorService { - public formatJson(input: string): string { - // Implementation - } -} - -// ❌ Avoid - implicit return types for public APIs -export function getEnvironmentOptions() { - // Return type is inferred - not ideal for public APIs -} -``` - -## Type Definitions +// ✅ unknown for truly unknown +const process = (data: unknown): string => + typeof data === 'string' ? data : JSON.stringify(data) -### Prefer `type` over `interface` +// ✅ any only allowed in constraint position +type ExtractPaths>> = { /* ... */ } -```typescript -// ✅ Good -type EditorOptions = { - theme: 'light' | 'dark'; - fontSize: number; -}; - -type JsonResult = { - data: T; - valid: boolean; -}; - -// ❌ Avoid -interface EditorOptions { - theme: string; - fontSize: number; -} +// ❌ any anywhere else +export function process(data: any): string { return data.toString() } ``` -### Component Props Types - -Define explicit prop types for all components: +Alternatives: `unknown` + type guard, generics, union types, utility types. -```typescript -// ✅ Good - explicit props type -type MonacoEditorProps = { - value: string; - onChange?: (value: string) => void; - language?: string; - readOnly?: boolean; -}; - -export const MonacoEditor = Shade({ - customElementName: 'monaco-editor', - render: ({ props }) => { - return ( -
    - {/* Editor implementation */} -
    - ); - }, -}); -``` +`@typescript-eslint/no-explicit-any` is **disabled** in `eslint.config.js` because the rule has no option for the constraint-position exception. Reviewers enforce the rule in code review. -## Type Guards +## Inference -### Provide Type Guards for Runtime Checks +Let TS infer internally. Be explicit at API boundaries, complex switch/case, or when inference is ambiguous. ```typescript -// ✅ Good - type guard -export function isValidJson(value: unknown): value is object { - if (typeof value !== 'string') return false; - try { - JSON.parse(value); - return true; - } catch { - return false; - } -} - -// Usage -if (isValidJson(data)) { - console.log('Valid JSON'); -} -``` - -## Observable Types - -### Type Observable Values +// ✅ inferred internally +const users = ['Alice', 'Bob'] +const count = users.length -Always provide explicit types for ObservableValue: +// ✅ explicit at exports +export const getUserById = async (id: string): Promise => { /* ... */ } -```typescript -// ✅ Good - explicit observable type -public currentTheme = new ObservableValue<'light' | 'dark'>('dark'); -public isLoading = new ObservableValue(false); -public editorContent = new ObservableValue(''); - -// ❌ Avoid - relying on inference for complex types -public currentTheme = new ObservableValue(null); // Type is ObservableValue +// ❌ noisy +const count: number = 5 ``` -## Generic Patterns +## Type guards -### Use Descriptive Generic Names +Use guards for runtime checks. Use them, not type assertions. ```typescript -// ✅ Good - descriptive generic names -type JsonParseResult = { - data: TData; - valid: boolean; - error?: string; -}; - -// ❌ Avoid - unclear generic names -type JsonParseResult = { - data: T; - valid: boolean; -}; -``` +// ✅ +export const isJsonObject = (value: unknown): value is Record => + typeof value === 'object' && value !== null && !Array.isArray(value) -### Constrain Generics When Appropriate +const isNonNullable = (value: T | null | undefined): value is T => + value !== null && value !== undefined -```typescript -// ✅ Good - constrained generic -export function formatValue(value: T): string { - return String(value); -} +values.filter(isNonNullable) // type-narrowed ``` -## Strict Null Checks +## Discriminated unions -### Handle Null and Undefined Explicitly +For multi-state values. Always include a `status` / `kind` discriminator. ```typescript -// ✅ Good - explicit null handling -export function parseJson(input: string): object | null { - try { - return JSON.parse(input); - } catch { - return null; - } -} - -// ✅ Good - use optional chaining -const theme = options?.theme ?? 'dark'; - -// ❌ Avoid - ignoring potential null -const theme = options.theme; // Error if options is null +// ✅ +type ValidationState = + | { status: 'idle' } + | { status: 'validating' } + | { status: 'valid'; document: unknown } + | { status: 'invalid'; errors: ValidationError[] } + +// ❌ no discriminator → cannot narrow +type Bad = { validating: boolean; errors?: ValidationError[]; document?: unknown } ``` -## Utility Types +## Observable typing -### Use TypeScript Utility Types +`ObservableValue` is invariant — provide explicit `T`, especially for nullable / union states. ```typescript -// ✅ Good - using utility types -type EditorOptions = { - theme: 'light' | 'dark'; - fontSize: number; - language: string; -}; - -// Partial for optional fields -type PartialEditorOptions = Partial; +// ✅ +public activeSchema = new ObservableValue(null) +public state = new ObservableValue({ status: 'idle' }) -// Pick for specific fields -type ThemeOption = Pick; - -// Omit for excluding fields -type EditorWithoutTheme = Omit; +// ❌ inference widens to ObservableValue +public activeSchema = new ObservableValue(null) ``` -## Type Inference +## Assertions -### Let TypeScript Infer When Possible +Avoid. Use type guards or `as const` instead. ```typescript -// ✅ Good - let TypeScript infer internal types -const users = ['Alice', 'Bob', 'Charlie']; // Inferred as string[] -const count = users.length; // Inferred as number - -// ✅ Good - explicit for complex conditional logic -export const getStatusDisplay = (status: string): JSX.Element | null => { - switch (status) { - case 'valid': - return
    Valid
    ; - case 'invalid': - return
    Invalid
    ; - default: - return null; - } -}; - -// ❌ Avoid - unnecessary annotations -const count: number = 5; -const name: string = 'John'; +// ✅ as const for literal types +const colors = ['red', 'green', 'blue'] as const +type Color = (typeof colors)[number] + +// ✅ assertion only when DOM API leaves no choice +const el = document.getElementById('x') as HTMLInputElement | null + +// ❌ dangerous +const user = unknownValue as User // prefer guard +const x = maybe! // prefer optional chaining ``` -## Summary +Avoid `!` non-null assertion. Prefer `?.` and `??`. + +## Utility types -**Key Principles:** +Reuse built-ins (`Partial`, `Pick`, `Omit`, `Record`, `Required`, `Readonly`). Export project-specific helpers when reused 2+ times. -1. **NEVER use `any`** - Use `unknown`, generics, or proper types -2. **Explicit types for exports** - Document the contract -3. **Prefer `type` over `interface`** -4. **Type component props** - Every Shade component should have typed props -5. **Type observables** - Always provide explicit types for ObservableValue -6. **Handle nulls** - Use strict null checks and optional chaining -7. **Use type guards** - For runtime type checking +## Double-cast anti-pattern -**Type Safety Checklist:** +Never `as unknown as T`. If you need a cast that strong, you're missing a type guard or a generic. Refactor. -- [ ] No `any` types anywhere -- [ ] All exported functions have explicit return types -- [ ] Component props are typed -- [ ] Observable values have explicit types -- [ ] Null/undefined handled explicitly +## Checklist (public API) -**Tools:** +- [ ] No `any` (except generic constraint exception) +- [ ] Explicit types on all exports +- [ ] Generic params named & constrained +- [ ] Type guards for runtime checks +- [ ] Discriminated unions for multi-state +- [ ] No `as unknown as T`, no `!` non-null +- [ ] `ObservableValue` types explicit -- Type checking: `yarn build` -- Strict mode: Enabled in `tsconfig.json` +Tools: `yarn build`, strict mode is on. diff --git a/.cursor/rules/rules-index.mdc b/.cursor/rules/rules-index.mdc index 9d49b62..d81f7a9 100644 --- a/.cursor/rules/rules-index.mdc +++ b/.cursor/rules/rules-index.mdc @@ -1,11 +1,18 @@ --- -name: Rules Index -description: Index of available rules and guidelines alwaysApply: true --- -This file contains a list of helpful information and context that the agent can reference when working in this codebase. Each entry provides specific guidance or rules for different aspects of the project. You can read these files using the readFile tool if the users prompt seems related. +Index of repository rules. Read the linked file when its topic is relevant to the user's prompt. -- [Code formatting, naming conventions, and file organization](./CODE_STYLE.mdc) -- [TypeScript type safety and strict typing guidelines](./TYPESCRIPT_GUIDELINES.mdc) -- [Unit testing with Vitest and E2E testing with Playwright](./TESTING_GUIDELINES.mdc) +- [Naming, formatting, imports, file/code organization](./CODE_STYLE.mdc) — auto-applied to all `.ts` / `.tsx` +- [Type safety, no `any`, generics, type guards, discriminated unions](./TYPESCRIPT_GUIDELINES.mdc) — auto-applied to all `.ts` / `.tsx` +- [Complexity thresholds for Shade components and services](./COMPLEXITY.mdc) — auto-applied; used in reviews and during implementation +- [Frontend Shade component patterns](./SHADES_COMPONENTS.mdc) — auto-applied in `frontend/` +- [Vitest unit + Playwright E2E patterns](./TESTING_GUIDELINES.mdc) — auto-applied to specs + +Skills (on-demand workflows): + +- `create-shade-component` — author a Shade component with proper hooks, styling, tests +- `write-tests` — author Vitest unit / Playwright E2E tests +- `fill-changelog` — fill changelog entries for the current branch +- `review-changes` — full code review with reviewer subagents diff --git a/.cursor/skills/create-shade-component/SKILL.md b/.cursor/skills/create-shade-component/SKILL.md new file mode 100644 index 0000000..ebfbf2f --- /dev/null +++ b/.cursor/skills/create-shade-component/SKILL.md @@ -0,0 +1,150 @@ +--- +name: create-shade-component +description: Create a Shade component from scratch. Use when the user asks to create/add a component, page, widget, dialog, form, or any UI primitive built with @furystack/shades. +--- + +# create-shade-component + +Generate a Shade component that follows `.cursor/rules/CODE_STYLE.mdc`, `.cursor/rules/TYPESCRIPT_GUIDELINES.mdc`, `.cursor/rules/COMPLEXITY.mdc`, and (where present) `.cursor/rules/SHADES_COMPONENTS.mdc` / `.cursor/rules/LIBRARY_DEVELOPMENT.mdc`. Read those rules first; this skill owns the workflow. + +## Step 1 — Clarify scope + +Ask the user (or infer): + +- **Role**: page (route target), composite (orchestrator), or presentational (leaf)? +- **Location**: `frontend/src/pages/.tsx` for pages, `frontend/src/components//index.tsx` for components with assets, `frontend/src/components/.tsx` otherwise. For framework code: `packages/shades-common-components/src/.tsx`. +- **`customElementName`**: kebab-case, unique, namespaced (e.g. `shade-login`, `app-bar`, `chat-message-list`). Verify uniqueness with a quick grep before creation. + +## Step 2 — Type the props + +```typescript +type MyComponentProps = { + // required first, optional after, callbacks last + id: string + variant?: 'compact' | 'full' + onSelect?: (id: string) => void +} +``` + +- `Props` suffix, PascalCase +- `type`, not `interface` +- ≤6 fields excluding `children` and `className` (COMPLEXITY threshold). Group into objects or split the component if you cross it. + +## Step 3 — Choose render hooks + +Pull only the hooks you actually use from the destructured `render` arg. + +| Need | Hook | +| --------------------------------------------- | ------------------------------------------------------- | +| Local state | `useState('key', initial)` — preferred for simple cases | +| Subscribe to a service `ObservableValue` | `useObservable('key', observable)` | +| Own a long-lived disposable in this component | `useDisposable('key', () => factoryFn())` | +| Direct DOM access (focus, scroll, animation) | `useRef('key')` | +| Set host element attributes / styles | `useHostProps({ ... })` | +| Get a service | `injector.get(ServiceToken)` | + +Forbidden / removed: + +- `element` in render args — use `useHostProps` / `useRef` +- `onAttach` / `onDetach` in `ShadeOptions` — use `useDisposable` +- Manual `.subscribe()` in render — use `useObservable` +- `.getValue()` in render without `useObservable` — re-renders won't trigger +- `useState` for hover / focus / active — use `&:hover` etc. in `css` + +## Step 4 — Choose styling approach + +| Use case | Where | +| ------------------------------------------------------------------ | --------------------------------------------------------------- | +| Defaults, pseudo-selectors (`&:hover`, `&:disabled`), nested rules | `css: { ... }` on `Shade(...)` | +| Per-instance dynamic values from props | `style={{ ... }}` on JSX, or `useHostProps({ style: { ... } })` | +| Theme color / spacing | `cssVariableTheme.text.primary` etc. (typed) | +| CSS custom properties for child consumption | `useHostProps({ style: { '--my-var': value } })` | + +Static styles → `css` (not `useHostProps({ style })`). Attribute-driven styles → CSS rule on the data attribute, not a JS branch. + +## Step 5 — Compose + +```typescript +import { Shade } from '@furystack/shades' + +type GreetingProps = { + name: string + onGreet?: () => void +} + +export const Greeting = Shade({ + customElementName: 'app-greeting', + css: { + padding: '8px 16px', + '&:hover': { backgroundColor: cssVariableTheme.background.paper }, + }, + render: ({ props, useState }) => { + const [count, setCount] = useState('count', 0) + const handleClick = () => { + setCount(count + 1) + props.onGreet?.() + } + return ( + + ) + }, +}) +``` + +## Step 6 — Apply COMPLEXITY guard + +Before finishing, count triggers from `.cursor/rules/COMPLEXITY.mdc`: + +- > 3 `useObservable` / `useDisposable` calls? +- > 4 distinct `injector.get(...)` calls? +- JSX return >60 lines? +- Render handler bodies >15 lines each? +- File length nearing 200 lines? + +If 2+ triggers approach their limit, **stop and decompose** before merging more logic in: + +- Extract pure helpers into a sibling file +- Pull a slice into a render-hook helper (`use-.ts`) +- Split into sub-Shades and pass focused props + +## Step 7 — Wire up + +If this is a route target, register it in the router (per repo: `frontend/src/components/body.tsx` or `routes/-routes.tsx`). For new feature entry points, the user may want one or more of: + +- Avatar menu item +- Command palette command provider +- App shortcut widget for the dashboard +- Default dashboard placement +- App bar entry (rarely) + +Ask the user which entry points are wanted; don't create them all by default. + +## Step 8 — Spec + +Run the `write-tests` skill (or follow `TESTING_GUIDELINES.mdc`) to create: + +- Vitest spec with `environment: 'jsdom'` for render tests +- Await `flushUpdates()` before asserting DOM after state changes +- E2E spec via Playwright if it's a user-facing flow; locate by `customElementName` + +## Step 9 — Verify + +```bash +yarn lint +yarn build +yarn test +``` + +Fix any `@furystack/eslint-plugin` violations; do not silence them with `eslint-disable` unless paired with `onDispose` (the documented exception for `prefer-using-wrapper`). + +## Output + +Report: + +- File(s) created +- Element name + render hooks used +- Props surface +- Tests added +- Any deferred decisions (entry points the user must confirm) diff --git a/.cursor/skills/review-changes/SKILL.md b/.cursor/skills/review-changes/SKILL.md index 84281f7..a285af2 100644 --- a/.cursor/skills/review-changes/SKILL.md +++ b/.cursor/skills/review-changes/SKILL.md @@ -51,6 +51,7 @@ git diff origin/master...HEAD --name-only | `reviewer-typescript` | Conditional | Skip ONLY if NO `.ts`/`.tsx` in `frontend/` changed | | `reviewer-eslint` | Conditional | Skip ONLY if NO `.ts`/`.tsx` in `frontend/` changed | | `reviewer-tests` | Conditional | Skip ONLY if NO `.ts`/`.tsx` in `frontend/` changed | +| `reviewer-complexity` | Conditional | Skip ONLY if NO `.ts`/`.tsx` in `frontend/` changed | **When in doubt, run the check.** Fast failures are better than missed issues. @@ -68,6 +69,7 @@ In one tool call batch, launch all applicable reviewers: - `reviewer-typescript` (if `.ts`/`.tsx` in `frontend/` changed) - `reviewer-eslint` (if `.ts`/`.tsx` in `frontend/` changed) - `reviewer-tests` (if `.ts`/`.tsx` in `frontend/` changed) +- `reviewer-complexity` (if `.ts`/`.tsx` in `frontend/` changed) **Note:** `reviewer-dependencies` checks changelog documentation but does NOT create/modify changelogs. If both changelog and dependency changes exist, both reviewers run in parallel - the dependency reviewer only reads existing changelogs. @@ -106,6 +108,10 @@ Check for: - Static `style` props in Shade definitions should use `css` instead +**Complexity Audit:** + +- Delegate to `reviewer-complexity` subagent to flag overgrown components and services introduced or worsened by the branch (heuristics from `.cursor/rules/COMPLEXITY.mdc`) + **Testing & Coverage:** - Delegate to `reviewer-tests` subagent to run unit tests and assess coverage @@ -142,6 +148,7 @@ Check for: - If `reviewer-versioning` passes → Do NOT mention it in the output - If `reviewer-changelog` passes → Do NOT mention it in the output - If `reviewer-dependencies` passes → Do NOT mention it in the output +- If `reviewer-complexity` passes → Do NOT mention it in the output Only report subagent findings when they detect actual problems. diff --git a/.cursor/skills/write-tests/SKILL.md b/.cursor/skills/write-tests/SKILL.md new file mode 100644 index 0000000..5f0f99f --- /dev/null +++ b/.cursor/skills/write-tests/SKILL.md @@ -0,0 +1,137 @@ +--- +name: write-tests +description: Author Vitest unit, Vitest integration, or Playwright E2E tests for FuryStack code. Use when the user asks to write/add/cover tests for a function, module, package, component, or flow. +--- + +# write-tests + +Generate tests that follow `.cursor/rules/TESTING_GUIDELINES.mdc` patterns. Read that rule first if not already in context — it owns the invariants (disposable safety, `flushUpdates`, minimal mocking, `injector.bind` over manual mocks). This skill owns the **workflow**. + +## Step 1 — Identify the target + +Ask the user (or infer from context) **what** to test: + +- File path or symbol name (e.g. `defineService`, `Header.tsx`, "the login flow") +- One unit or many? +- Existing spec or new file? + +If unclear, ask one short clarifying question before proceeding. + +## Step 2 — Pick the test type + +| Type | When | Runner | File pattern | +| ----------- | ------------------------------------------------------- | ---------- | ----------------------------------------------------------- | +| Unit | One function / class / hook helper / Shade in isolation | Vitest | `.spec.ts` / `.spec.tsx` co-located | +| Integration | Multiple modules wired through a real `Injector` | Vitest | `.integration.spec.ts` co-located (framework only) | +| E2E | User-facing flow through running app + browser | Playwright | `e2e/.e2e.spec.ts` | + +Skip the E2E branch if the repo has no `e2e/` folder or no Playwright config. + +## Step 3 — Read the source + +Read the file under test. Identify: + +- Public exports (functions, classes, types, components) +- Branches / edge cases (null, undefined, errors, async failures) +- Disposable resources owned (`ObservableValue`, `Cache`, `Injector`, stores) +- DI dependencies used inside (`inject(...)`, `injector.get(...)`) +- For Shades components: render hooks used, observables subscribed, async behaviors + +## Step 4 — Scaffold the spec + +### Unit (Vitest) + +Always wrap disposables in `using` / `usingAsync`. Substitute deps via `injector.bind`, not `vi.fn()` mocks (unless the dep is a non-DI external). + +```typescript +import { describe, expect, it } from 'vitest' +import { createInjector } from '@furystack/inject' +import { usingAsync } from '@furystack/utils' + +describe('', () => { + it(' when ', async () => { + await usingAsync(createInjector(), async (injector) => { + // arrange / act / assert + }) + }) +}) +``` + +For Shade components, set `environment: 'jsdom'` in vitest project config (already done in the apps). Render with `Shade`, await `flushUpdates()` before asserting DOM. + +### Integration (Vitest, framework only) + +Use real services across packages, real `Injector`, real stores (`InMemoryStore`). File: `*.integration.spec.ts`. Same `usingAsync` discipline. + +### E2E (Playwright) + +```typescript +import { expect, test } from '@playwright/test' + +test.describe('', () => { + test('', async ({ page }) => { + await page.goto('/') + await expect(page.locator('')).toBeVisible() + // interact + assert + }) +}) +``` + +Locate Shade components by their `customElementName` (e.g. `page.locator('shade-app-header')`). Prefer semantic locators (`getByRole`, `getByLabel`) inside. + +## Step 5 — Cover required cases + +For each public export, ensure tests for: + +- **Happy path** — normal inputs, expected output +- **Documented branches** — every branch the function explicitly handles (modes, options, discriminator values) +- **Error paths** — thrown errors, rejected promises, `RequestError` codes that the function emits intentionally +- **Disposal** — for owners of `ObservableValue` / `Cache` / `Injector`, assert post-dispose behavior as documented + +**Why this list does not include "null / undefined / random invalid inputs":** older guidance asked for blanket null/undefined tests on every export. That asserts TypeScript works, not that the code does, and it encourages defensive runtime guards for inputs the types already forbid. Trust the TS type system; consumers will use it too. + +You **MUST** test null / undefined / malformed inputs when, and only when: + +- The function sits at an **external boundary without schema validation** — REST handler not wrapped in `Validate(...)`, WebSocket message handler, file parser, env-var consumer, anything consuming `unknown` or `JSON.parse` output. At runtime there is no contract enforcement, so the function is the line of defense and bad-payload tests are required. +- The function's **type signature itself allows** the case (`T | null`, `T | undefined`, `unknown`, optional `T?`). Then the documented behavior on each branch must be exercised. + +If a `Validate(...)` (or equivalent JSON-schema / zod) wrapper sits ahead of the function, the wrapper covers the bad-payload tests — the inner function should not retest them. + +If you can't cover a path because the source has dead code, surface it instead of writing a vacuous test. + +## Step 6 — Watch for false positives + +- Assertions inside `try/catch` → use `expect.assertions(N)` so the test fails if the function unexpectedly resolves. +- Assertions inside `.forEach` / `.map` callbacks → same risk. +- Async assertions without `await` → promise resolves after the test completes. +- Mocks that always return success → make at least one test exercise failure. + +## Step 7 — Run + +| Type | Command | +| --------------- | --------------------------------- | +| Unit | `yarn test` | +| Specific file | `yarn test ` | +| Coverage | `yarn test --coverage` | +| E2E | `yarn test:e2e` | +| E2E single file | `yarn playwright test e2e/` | + +Investigate every failure. Do **not** mark a test `skip` to make CI green. + +## Step 8 — Verify lint + types + +```bash +yarn lint +yarn build +``` + +Fix anything new the change introduced. + +## Output + +Report: + +- Files created / modified +- Number of cases added per export +- Coverage delta if `--coverage` was run +- Any uncoverable paths with rationale diff --git a/.vscode/settings.json b/.vscode/settings.json index 4068ddf..eec3df4 100644 --- a/.vscode/settings.json +++ b/.vscode/settings.json @@ -27,5 +27,6 @@ }, "[html]": { "editor.defaultFormatter": "esbenp.prettier-vscode" - } + }, + "vitest.disableWorkspaceWarning": true } diff --git a/.yarn/versions/174d6b23.yml b/.yarn/versions/174d6b23.yml new file mode 100644 index 0000000..3f5c8a0 --- /dev/null +++ b/.yarn/versions/174d6b23.yml @@ -0,0 +1,2 @@ +declined: + - json-tools