From 051efef4efce5cab8b97bf34e26615deecd26faf Mon Sep 17 00:00:00 2001 From: Michal Murawski Date: Wed, 25 Mar 2026 14:58:40 +0100 Subject: [PATCH] chore: update curosr to respect browser tests --- .cursor/notepads/add-play-tests.md | 13 --- .cursor/rules/browser-tests.mdc | 48 ++++++++ .cursor/rules/code-review.mdc | 68 +++++++---- .cursor/rules/design-system.mdc | 49 +------- .cursor/rules/react-patterns.mdc | 42 +------ .cursor/rules/standards.mdc | 59 +--------- .cursor/rules/storybook.mdc | 79 ++++--------- .cursor/skills/component-scaffold/SKILL.md | 50 +++++--- .cursor/skills/migrate-story-tests/SKILL.md | 119 ++++++++++++++++++++ .cursor/skills/pr-prep/SKILL.md | 44 ++------ .cursor/skills/rca-debug/SKILL.md | 60 ++++++++++ README.md | 52 +++++---- cspell.json | 1 + 13 files changed, 379 insertions(+), 305 deletions(-) delete mode 100644 .cursor/notepads/add-play-tests.md create mode 100644 .cursor/rules/browser-tests.mdc create mode 100644 .cursor/skills/migrate-story-tests/SKILL.md create mode 100644 .cursor/skills/rca-debug/SKILL.md diff --git a/.cursor/notepads/add-play-tests.md b/.cursor/notepads/add-play-tests.md deleted file mode 100644 index 35ccc526c..000000000 --- a/.cursor/notepads/add-play-tests.md +++ /dev/null @@ -1,13 +0,0 @@ -## Add Play Tests to Stories - -Look at the current file (or the file I specify). Find all exported stories that are missing a `play` function. - -For each story without `play`, generate an interaction test following these rules: - -1. Import `{ expect, fn, userEvent, within }` from `'storybook/test'` -2. Use `fn()` for all callback args in the story's `args` -3. Use a11y queries only: `getByRole`, `getByLabelText`, `getByText` -- never `getByTestId` -4. Use semantic assertions: `toBeChecked()`, `toBeDisabled()`, `toBeVisible()`, `toBeInTheDocument()` -5. Await all interactions: `await userEvent.click(...)`, `await expect(...)` -6. Don't test implementation details (no data attributes, no CSS selectors), unless it's a required part of the story -7. Test user-visible behavior: render check, click interactions, callback assertions diff --git a/.cursor/rules/browser-tests.mdc b/.cursor/rules/browser-tests.mdc new file mode 100644 index 000000000..4254005e0 --- /dev/null +++ b/.cursor/rules/browser-tests.mdc @@ -0,0 +1,48 @@ +--- +description: Vitest browser test patterns for design system components +globs: **/__tests__/*.browser.test.tsx +--- + +# Vitest Browser Tests + +Behavioral coverage for design-system components lives next to the component under `__tests__/`, e.g. `ds-checkbox/__tests__/ds-checkbox.browser.test.tsx`. Storybook stories are for docs and controls only. + +## Principles + +1. **Stories document UI**; assert behavior in `*.browser.test.tsx`. +2. **Spy callbacks with `vi.fn()`** in tests (not Storybook `fn()`). +3. **Prefer a11y queries**: `page.getByRole`, `getByLabelText`, `getByText` -- avoid `getByTestId` unless unavoidable. +4. **Use Vitest browser matchers**: `await expect.element(locator).toBeChecked()`, `toBeDisabled()`, `toBeVisible()`, etc. +5. **Await async UI**: `await locator.click()`, `await expect.element(...)`. +6. **Test user-visible behavior**, not implementation details; use `data-*` only when it is an explicit contract (e.g. indeterminate state). +7. **Disabled / blocked interaction**: `await checkbox.click({ force: true })` when asserting a disabled control does not change state. +8. **Controlled component testing**: define an inline wrapper with `useState` to test controlled behavior. +9. **Rerender for prop changes**: use `const { rerender } = await page.render(...)` to test conditional rendering. +10. **Hover interactions**: `await locator.hover()` / `await locator.unhover()` for tooltip-style components. +11. **Nested locators**: scope queries within a parent: `parentLocator.getByText(...)`. +12. **`toBeInTheDocument` vs `toBeVisible`**: use `not.toBeInTheDocument()` for removed DOM elements, `not.toBeVisible()` for hidden ones. +13. **Querying disabled elements**: pass `{ disabled: true }` to role queries: `page.getByRole('checkbox', { disabled: true })`. + +## Example: callback spy + +```tsx +import { describe, expect, it, vi } from 'vitest'; +import { page } from 'vitest/browser'; +import DsWidget from '../ds-widget'; + +describe('DsWidget', () => { + it('calls onSave when submitted', async () => { + const onSave = vi.fn(); + await page.render(); + + await page.getByRole('button', { name: /save/i }).click(); + expect(onSave).toHaveBeenCalledOnce(); + }); +}); +``` + +## Run + +```bash +pnpm --filter @drivenets/design-system test packages/design-system/src/components/ds-{name}/__tests__/ds-{name}.browser.test.tsx --run +``` diff --git a/.cursor/rules/code-review.mdc b/.cursor/rules/code-review.mdc index 11e643131..d4cc1f6dc 100644 --- a/.cursor/rules/code-review.mdc +++ b/.cursor/rules/code-review.mdc @@ -1,19 +1,40 @@ --- -description: Apply when reviewing code changes, running git diff, or preparing a PR +description: Apply when preparing a PR, reviewing code changes, running git diff, or generating a changeset alwaysApply: false --- -# Code Review Rules +# PR Workflow & Code Review -It is a local code review process done before submitting a PR. +## Git & PR Workflow -## Process +| Requirement | Details | +| --- | --- | +| **No force-push after review** | GitHub loses review history; use merge commits instead | +| **Changelog required** | Run `pnpm changelog` before merge | +| **Conventional commits** | PR title must follow conventional commit format | +| **Separate concerns** | Unrelated changes go in separate PRs; easier revert and isolation | +| **Changeset messages are user-facing** | Write "Add X to Y" or "Fix X in Y", not implementation details | +| **`skip changelog` label** | Use only for non-user-facing changes (CI, tests, docs); never for bug fixes | +| **Use `--frozen-lockfile`** | Run `pnpm install --frozen-lockfile` to avoid unnecessary lockfile changes | +| **Deprecation rules** | Add to `@drivenets/eslint-plugin-design-system` when deprecating | + +```markdown + +Add DsCard component + + +Refactor card to use CSS modules and fix hover state selector specificity +``` + +--- + +## Code Review Process 1. Get diff: `git diff origin/main` -2. Review every changed file against project cursor rules +2. Review every changed file against project cursor rules (scss, react-patterns, storybook, etc.) 3. Flag only clear, high-severity issues (max 10 inline comments) -## Inline comment format +### Inline comment format ```typescript /** @@ -23,19 +44,24 @@ It is a local code review process done before submitting a PR. - One issue per comment; place on the exact changed line - Natural tone, specific and actionable; do not mention automated or high-confidence -- Severity emojis: 🚨 Critical πŸ”’ Security ⚑ Performance ⚠️ Logic ✨ Improvement - -## Priorities to check - -- No `!important` in SCSS -- No hardcoded colors or spacing (use `--color-*`, `--spacing-*` tokens) -- No `:global` in CSS modules -- No unnecessary `useMemo`/`useCallback` -- No `forwardRef` (deprecated) -- No cross-component internal imports -- No `useLayoutEffect` without DOM reads -- Stories have `play` functions with `fn()` and a11y queries -- No inline styles in stories -- Props layer doesn't expose library internals -- Logic errors, security issues, race conditions, missing edge cases +- Severity: 🚨 Critical πŸ”’ Security ⚑ Performance ⚠️ Logic ✨ Improvement + +--- + +## PR Checklist + +Before submitting a PR: +- [ ] Changelog added (`pnpm changelog`) with user-facing message +- [ ] Behavioral coverage in `*.browser.test.tsx` where interactions matter +- [ ] Browser tests use a11y queries (no test-ids unless unavoidable) +- [ ] CSS uses design tokens, no `!important` +- [ ] No inline styles in stories +- [ ] Props layer doesn't expose library internals +- [ ] Types exported from `.types.ts` with variant arrays +- [ ] Code is well-spaced and readable +- [ ] Matches Figma design +- [ ] Storybook examples show all states (controlled + localized) +- [ ] No cross-component internal imports +- [ ] No unnecessary `useMemo`/`useCallback` +- [ ] Check Ark UI for existing primitives before custom implementation diff --git a/.cursor/rules/design-system.mdc b/.cursor/rules/design-system.mdc index 98499435c..efe86ad15 100644 --- a/.cursor/rules/design-system.mdc +++ b/.cursor/rules/design-system.mdc @@ -1,9 +1,9 @@ --- -description: Component scaffolding templates for creating new design system components +description: Design system component conventions and primitive library choices globs: packages/design-system/src/components/**/* --- -# Component Scaffolding +# Design System Components ## File Structure @@ -18,51 +18,6 @@ ds-{name}/ └── ds-{name}.stories.tsx # Stories ``` -## Implementation Template - -```tsx -import classNames from 'classnames'; -import styles from './ds-{name}.module.scss'; -import type { Ds{Name}Props } from './ds-{name}.types'; - -/** - * Design system {Name} component - */ -const Ds{Name} = ({ - className, - children, - ...props -}: Ds{Name}Props) => { - return ( -
- {children} -
- ); -}; - -export default Ds{Name}; -``` - -## Types Template - -```typescript -import type { ComponentPropsWithoutRef, ReactNode } from 'react'; - -export interface Ds{Name}Props extends ComponentPropsWithoutRef<'div'> { - /** - * JSDoc description - */ - label?: ReactNode; -} -``` - -## Index Barrel - -```typescript -export { default as Ds{Name} } from './ds-{name}'; -export type { Ds{Name}Props } from './ds-{name}.types'; -``` - ## Primitive Libraries 1. **Ark UI** (`@ark-ui/react`) - Primary choice for new components diff --git a/.cursor/rules/react-patterns.mdc b/.cursor/rules/react-patterns.mdc index e5d7fd115..46a80c2f9 100644 --- a/.cursor/rules/react-patterns.mdc +++ b/.cursor/rules/react-patterns.mdc @@ -87,21 +87,7 @@ const id = useId(); container.addEventListener('scroll', checkOverflow, { passive: true }); ``` -One other situation you might want to use useLayoutEffect instead of useEffect is if you're updating a value (like a ref) -and you want to make sure it's up-to-date before any other code runs. For example: - -```tsx -// Good -const ref = React.useRef() -React.useEffect(() => { - ref.current = 'some value' -}) - -// then, later in another hook or something -React.useLayoutEffect(() => { - console.log(ref.current) // <-- this logs an old value because this runs first! -}) -``` +Also use `useLayoutEffect` when updating a ref that must be current before other hooks read it (it fires before `useEffect`). --- @@ -144,29 +130,3 @@ const [inputValue, setInputValue] = useState(''); ``` ---- - -## Component API Ordering - -| Requirement | Details | -| -------------------------------------------- | -------------------------------------------------------------------------------- | -| **Value props first, callbacks last** | Order: value/config props, then render/slot props, then callbacks | -| **`locale` prop for i18n** | Components with hardcoded text should accept a `locale` prop for overrides | - -```typescript -// Good - ordered interface -export interface DsTagFilterProps { - // Value props - items: TagFilterItem[]; - selectedKeys?: string[]; - size?: 'small' | 'medium'; - - // Slot props - slotProps?: { tag?: Partial }; - locale?: { clearAll?: string; showMore?: string }; - - // Callbacks - onSelectionChange?: (keys: string[]) => void; - onClear?: () => void; -} -``` diff --git a/.cursor/rules/standards.mdc b/.cursor/rules/standards.mdc index 58902021d..9e3f42535 100644 --- a/.cursor/rules/standards.mdc +++ b/.cursor/rules/standards.mdc @@ -1,35 +1,9 @@ --- -description: PR requirements and code standards for the design-system repository +description: Code standards for the design-system repository alwaysApply: true --- -# Pull Request Requirements & Code Standards - -Based on code review feedback, here are the consolidated standards for the design-system repository. - ---- - -## Git & PR Workflow - -| Requirement | Details | -| -------------------------------------------- | -------------------------------------------------------------------------------- | -| **No force-push after review** | GitHub loses review history; use merge commits instead | -| **Changelog required** | Run `pnpm changelog` before merge | -| **Conventional commits** | PR title must follow conventional commit format | -| **Separate concerns** | Unrelated changes go in separate PRs; easier revert and isolation | -| **Changeset messages are user-facing** | Write "Add X to Y" or "Fix X in Y", not implementation details | -| **`skip changelog` label** | Use only for non-user-facing changes (CI, tests, docs); never for bug fixes | -| **Use `--frozen-lockfile`** | Run `pnpm install --frozen-lockfile` to avoid unnecessary lockfile changes | - -```markdown - -Add DsCard component - - -Refactor card to use CSS modules and fix hover state selector specificity -``` - ---- +# Code Standards ## Component API Design @@ -42,6 +16,8 @@ Refactor card to use CSS modules and fix hover state selector specificity | **Consistent callback naming** | Prefer `onXChange` pattern | | **`null` for empty callback values** | Use `null` (not `undefined`) for optional values in callbacks: `(value: string \| null) => void` | | **Single deprecation comment** | In types file only, not on every export | +| **Value props first, callbacks last** | Order: value/config props, then render/slot props, then callbacks | +| **`locale` prop for i18n** | Components with hardcoded text should accept a `locale` prop for overrides | ```typescript // Good - own props layer with null for clearable value @@ -145,30 +121,3 @@ import { formatDate } from '../../utils/format-date'; | **Barrel exports** | Use `index.ts` not `index.tsx` | | **kebab-case files** | All files must be kebab-case | ---- - -## ESLint Plugin - -| Requirement | Details | -| -------------------------------------------- | -------------------------------------------------------------------------------- | -| **Deprecation rules** | Add to `@drivenets/eslint-plugin-design-system` when deprecating | - ---- - -## PR Checklist - -Before submitting a PR: - -- [ ] Changelog added (`pnpm changelog`) with user-facing message -- [ ] All stories have `play` tests with `fn()` for callbacks -- [ ] Tests use a11y queries (no test-ids) -- [ ] CSS uses design tokens, no `!important` -- [ ] No inline styles in stories -- [ ] Props layer doesn't expose library internals -- [ ] Types exported from `.types.ts` with variant arrays -- [ ] Code is well-spaced and readable -- [ ] Matches Figma design -- [ ] Storybook examples show off all states (including controlled + localized) -- [ ] No cross-component internal imports -- [ ] No unnecessary `useMemo`/`useCallback` -- [ ] Check Ark UI for existing primitives before custom implementation diff --git a/.cursor/rules/storybook.mdc b/.cursor/rules/storybook.mdc index 6676f21ae..e3b4d0d45 100644 --- a/.cursor/rules/storybook.mdc +++ b/.cursor/rules/storybook.mdc @@ -1,60 +1,15 @@ --- -description: Storybook story and interaction test standards +description: Storybook story layout, args, and styling β€” behavior is covered in Vitest browser tests globs: **/*.stories.tsx --- # Storybook Standards -## Interaction Tests +## Behavior tests -| Requirement | Details | -| -------------------------------------------- | -------------------------------------------------------------------------------- | -| **Every story needs a `play` function** | Stories without `play` are untested; add at minimum a render assertion | -| **Use `fn()` for callback spying** | Import from `@storybook/test`; pass as args for action logs + test assertions | -| **Use a11y queries** | `getByRole`, `getByLabelText`, `getByText` over `getByTestId` | -| **Semantic assertions** | `toBeChecked()`, `toBeDisabled()`, `toBeVisible()` over `toHaveAttribute` | -| **Don't test implementation details** | No data attributes, CSS selectors, or internal state; test user-visible behavior | -| **Await all interactions** | `await userEvent.click(...)`, `await expect(...)` | - -```tsx -// Good -export const Default: Story = { - args: { - onSelect: fn(), - onChange: fn(), - }, - play: async ({ canvasElement, args }) => { - const canvas = within(canvasElement); - const button = canvas.getByRole('button', { name: /submit/i }); - - await userEvent.click(button); - await expect(args.onSelect).toHaveBeenCalledOnce(); - }, -}; - -// Bad - no play function, no fn(), testing implementation details -export const Default: Story = { - args: { - onSelect: () => console.log('selected'), - }, - // no play function = no test -}; -``` - -For stories with callback args, verify the callback was called: - -```tsx -await userEvent.click(button); -await expect(args.onClick).toHaveBeenCalledOnce(); -``` - -For stories showing disabled state, verify interactions are blocked: - -```tsx -await userEvent.click(element, { pointerEventsCheck: 0 }); -await expect(args.onClick).not.toHaveBeenCalled(); -``` +Interaction and regression coverage lives in **`*.browser.test.tsx`** next to the component (`__tests__/`), using Vitest browser mode (`vitest/browser`), `page.render`, and `expect.element`. Do not rely on Storybook `play` functions. +See `.cursor/rules/browser-tests.mdc` for patterns (a11y queries, `vi.fn()` spies, semantic assertions). --- @@ -65,20 +20,32 @@ await expect(args.onClick).not.toHaveBeenCalled(); | **Use `mockdate` for date-dependent stories**| `vi.useFakeTimers` breaks Ark UI internals (setTimeout, setInterval) | | **Add reference comment** | Explain why `mockdate` is used over `vi.useFakeTimers` on first usage in file | +Use a **decorator** so the fixed date applies while the story is mounted (not inside a removed `play` function): + ```tsx +import { useEffect } from 'react'; import MockDate from 'mockdate'; +import type { Decorator, Meta } from '@storybook/react-vite'; +import Ds{Name} from './ds-{name}'; -export const WithDate: Story = { - // mockdate is used instead of vi.useFakeTimers because Ark UI - // internally uses setTimeout/setInterval which hang with fake timers - play: async ({ canvasElement }) => { +// mockdate is used instead of vi.useFakeTimers because Ark UI +// internally uses setTimeout/setInterval which hang with fake timers +const withFixedDate: Decorator = (Story) => { + useEffect(() => { MockDate.set('2026-01-15'); - // ... test logic - MockDate.reset(); - }, + return () => MockDate.reset(); + }, []); + return ; +}; + +const meta: Meta = { + component: Ds{Name}, + decorators: [withFixedDate], }; ``` +For assertions on date-dependent behavior, prefer **`*.browser.test.tsx`** with the same `mockdate` approach if needed. + --- ## Story Variants diff --git a/.cursor/skills/component-scaffold/SKILL.md b/.cursor/skills/component-scaffold/SKILL.md index 1cff27fb9..0ae8bc3ca 100644 --- a/.cursor/skills/component-scaffold/SKILL.md +++ b/.cursor/skills/component-scaffold/SKILL.md @@ -1,6 +1,6 @@ --- name: component-scaffold -description: Scaffold a new design system component with all required files, Ark UI integration, stories with play tests, and barrel export wiring. Use when the user asks to create, scaffold, or add a new component. +description: Scaffold a new design system component with all required files, Ark UI integration, Storybook stories (docs/controls), optional Vitest browser tests, and barrel export wiring. Use when the user asks to create, scaffold, or add a new component. --- # Component Scaffold Skill @@ -30,9 +30,9 @@ Before writing any code, check if Ark UI already provides this component: - Do NOT duplicate Ark internal state with `useState`. 3. If no matching component exists, build a custom implementation. -### Step 2: Create the 5 component files +### Step 2: Create the component files -All files go in `packages/design-system/src/components/ds-{name}/`. +All files go in `packages/design-system/src/components/ds-{name}/` (plus optional `__tests__/` for browser tests). #### `ds-{name}.types.ts` @@ -124,7 +124,6 @@ Follow these rules: ```tsx import type { Meta, StoryObj } from '@storybook/react-vite'; -import { expect, fn, userEvent, within } from 'storybook/test'; import Ds{Name} from './ds-{name}'; import { ds{Name}Variants } from './ds-{name}.types'; @@ -147,28 +146,41 @@ export default meta; type Story = StoryObj; export const Default: Story = { - args: { - onChange: fn(), - }, - play: async ({ canvasElement, args }) => { - const canvas = within(canvasElement); - // Add assertions using getByRole, getByText, getByLabelText - // Use semantic assertions: toBeChecked, toBeDisabled, toBeVisible - // Await all interactions - }, + args: {}, }; ``` Follow these rules: -- Every story MUST have a `play` function. -- Use `fn()` for all callback args. -- Use a11y queries: `getByRole`, `getByLabelText`, `getByText`. Never `getByTestId`. +- Stories are for documentation and controls; **behavior is tested in `*.browser.test.tsx`** (see below). - Import variant arrays from types for `argTypes.options`. - Hide internal args (`className`, `style`, `ref`) with `table: { disable: true }`. - No inline styles -- use `*.stories.module.scss` if needed. - Add stories for: Default, each variant, Disabled, Controlled (if applicable), Localized (if has `locale` prop). +#### `__tests__/ds-{name}.browser.test.tsx` (recommended) + +Use Vitest browser mode (`vitest/browser`). Spy callbacks with `vi.fn()`; query with `page.getByRole`, `getByLabelText`, `getByText` (not `getByTestId` unless unavoidable). + +```tsx +import { describe, expect, it, vi } from 'vitest'; +import { page } from 'vitest/browser'; +import Ds{Name} from '../ds-{name}'; + +describe('Ds{Name}', () => { + it('renders and responds to interaction', async () => { + const onChange = vi.fn(); + await page.render(); + + // await expect.element(page.getByRole('...')).toBeVisible(); + // await page.getByRole('button').click(); + // expect(onChange).toHaveBeenCalled(); + }); +}); +``` + +See existing examples under `packages/design-system/src/components/*/__tests__/*.browser.test.tsx`. + #### `index.ts` ```typescript @@ -195,4 +207,10 @@ pnpm eslint packages/design-system/src/components/ds-{name}/ pnpm --filter @drivenets/design-system typecheck ``` +If you added `__tests__/ds-{name}.browser.test.tsx`: + +```bash +pnpm --filter @drivenets/design-system test packages/design-system/src/components/ds-{name}/__tests__/ds-{name}.browser.test.tsx --run +``` + Fix any errors before finishing. diff --git a/.cursor/skills/migrate-story-tests/SKILL.md b/.cursor/skills/migrate-story-tests/SKILL.md new file mode 100644 index 000000000..439735578 --- /dev/null +++ b/.cursor/skills/migrate-story-tests/SKILL.md @@ -0,0 +1,119 @@ +--- +name: migrate-story-tests +description: Migrate Storybook play functions to Vitest browser tests. Use when the user says "migrate tests for ds-X", "move play to browser tests", or "convert story tests". +--- + +# Migrate Story Tests Skill + +Converts Storybook `play` functions into `*.browser.test.tsx` files using Vitest browser mode, then cleans up the story file. + +## API Translation + +| Storybook (`storybook/test`) | Vitest browser (`vitest`, `vitest/browser`) | +| ------------------------------------------------ | ----------------------------------------------------------- | +| `within(canvasElement)` | `page` (top-level, no scoping needed) | +| `canvas.getByRole('button', { name })` | `page.getByRole('button', { name })` | +| `canvas.getByText('...')` | `page.getByText('...')` | +| `userEvent.click(el)` | `await locator.click()` | +| `userEvent.click(el, { pointerEventsCheck: 0 })` | `await locator.click({ force: true })` | +| `userEvent.type(el, 'text')` | `await locator.fill('text')` | +| `userEvent.clear(el)` | `await locator.clear()` | +| `userEvent.hover(el)` | `await locator.hover()` | +| `userEvent.unhover(el)` | `await locator.unhover()` | +| `userEvent.keyboard('{Enter}')` | `await locator.press('Enter')` | +| `expect(el).toBeChecked()` | `await expect.element(locator).toBeChecked()` | +| `expect(el).toBeDisabled()` | `await expect.element(locator).toBeDisabled()` | +| `expect(el).toBeInTheDocument()` | `await expect.element(locator).toBeInTheDocument()` | +| `expect(el).toBeVisible()` | `await expect.element(locator).toBeVisible()` | +| `expect(el).toHaveAttribute(k, v)` | `await expect.element(locator).toHaveAttribute(k, v)` | +| `expect(el).toHaveTextContent('...')` | `await expect.element(locator).toHaveTextContent('...')` | +| `waitFor(() => expect(...))` | Direct `await` (Vitest locators auto-retry) | +| `fn()` (in story args for Actions tab) | `vi.fn()` in test; keep `fn()` in stories for Actions tab | +| `{ args }` from play context | Props passed directly to `page.render()` | + +## Steps + +### Step 1: Identify play functions + +Find all `play:` entries in the target story file: + +```bash +rg 'play\s*:' packages/design-system/src/components/ds-{name}/ds-{name}.stories.tsx +``` + +For each story with a `play` function, note: + +- The story name (e.g. `Default`, `Controlled`, `Disabled`) +- The `args` and `render` function if present +- What the play function asserts + +### Step 2: Create or extend browser test file + +Target: `packages/design-system/src/components/ds-{name}/__tests__/ds-{name}.browser.test.tsx` + +If the file already exists, merge new tests without duplicating existing coverage. + +Structure: + +```tsx +import { useState } from 'react'; +import { describe, expect, it, vi } from 'vitest'; +import { page } from 'vitest/browser'; +import DsName from '../ds-name'; + +describe('DsName', () => { + it('should ...', async () => { + await page.render(); + // assertions + }); +}); +``` + +Translation rules: + +1. Each `play` function becomes one or more `it()` blocks grouped by behavior. +2. Replace `within(canvasElement)` scoping with `page` -- locators are global. +3. Replace `userEvent.click(el)` with `await locator.click()`. +4. Replace `expect(el).toBeX()` with `await expect.element(locator).toBeX()`. +5. Remove `waitFor` wrappers -- Vitest locators auto-retry. +6. For controlled stories with `render`, inline a wrapper component using `useState` inside the test. +7. For disabled interactions, use `{ force: true }` instead of `{ pointerEventsCheck: 0 }`. +8. For disabled element queries, add `{ disabled: true }` to role options. +9. Replace `fn()` with `vi.fn()` for callback spies. + +### Step 3: Clean up story file + +After migrating all play functions: + +1. **Remove all `play` properties** from every story object. +2. **Remove stale imports** -- if no `play` functions remain, remove `expect`, `userEvent`, `waitFor`, `within` from the `storybook/test` import. Keep `fn` if it's used in `args` for the Actions panel. +3. **Keep stories intact** -- do not remove stories themselves; they still serve as visual documentation. +4. **Preserve render functions** -- `render` in stories is for Storybook UI, not tests. + +### Step 4: Verify + +```bash +# Run the new browser tests +pnpm --filter @drivenets/design-system test packages/design-system/src/components/ds-{name}/__tests__/ds-{name}.browser.test.tsx --run + +# Lint the changed files +pnpm eslint packages/design-system/src/components/ds-{name}/ + +# Typecheck +pnpm --filter @drivenets/design-system typecheck +``` + +All three must pass before the migration is complete. + +### Step 5: Report + +List what was migrated: + +``` +Migrated ds-{name}: +- βœ“ Default: toggle checked state β†’ should toggle checked state when clicked +- βœ“ Controlled: controlled toggle β†’ should support controlled checked state +- βœ“ Disabled: disabled interaction β†’ should not toggle when disabled +- Removed play from: Default, Controlled, Disabled, CustomLabel +- Cleaned imports: removed expect, userEvent, waitFor, within from storybook/test +``` diff --git a/.cursor/skills/pr-prep/SKILL.md b/.cursor/skills/pr-prep/SKILL.md index d41436860..2aa89f932 100644 --- a/.cursor/skills/pr-prep/SKILL.md +++ b/.cursor/skills/pr-prep/SKILL.md @@ -5,7 +5,7 @@ description: Prepare a PR for submission by running all checks, validating stori # PR Preparation Skill -Automate the pre-submission checklist from `standards.mdc`. +Automate the pre-submission checklist from `code-review.mdc`. ## Steps @@ -21,6 +21,7 @@ Categorize files by type: - `*.stories.tsx` -- Story files - `*.module.scss` -- Style files - `*.test.ts` / `*.test.tsx` -- Test files +- `*.browser.test.tsx` -- Vitest browser tests (live next to components under `__tests__/`) - `.changeset/*.md` -- Changeset files ### Step 2: Run checkers on changed files only @@ -45,31 +46,13 @@ pnpm --filter @drivenets/design-system typecheck pnpm --filter @drivenets/design-system test --run ``` -### Step 3: Validate story files +For new components or substantial interaction/behavior changes, confirm an updated `*.browser.test.tsx` exists under the component’s `__tests__/` when appropriate. -For every `.stories.tsx` in the diff: +### Step 3: Review changed files against project rules -1. Check that every exported story has a `play` function. Search for `export const` declarations and verify each has `play:` or `play :`. -2. Check that callback args use `fn()` not inline functions. -3. Check for inline `style=` attributes -- should use `*.stories.module.scss` instead. +For each changed file, apply the relevant cursor rules (`scss.mdc`, `react-patterns.mdc`, `storybook.mdc`, `standards.mdc`). Flag violations found in the diff. -### Step 4: Validate SCSS files - -For every `.module.scss` in the diff: - -1. Search for `!important` -- flag any occurrence. -2. Search for hardcoded colors (hex values like `#xxx`) -- flag any not in a comment. -3. Search for `:global` -- flag any occurrence. - -### Step 5: Validate TypeScript files - -For every `.tsx` / `.ts` in the diff: - -1. Check for cross-component imports (importing from `../ds-other-component/`) -- flag as violation. Only allow main components imports, not utilities or subcomponents. -2. Check for `forwardRef` usage -- flag as deprecated. -3. Check for `vi.useFakeTimers` in story files -- should use `mockdate`. - -### Step 6: Check changeset +### Step 4: Check changeset ```bash ls .changeset/*.md 2>/dev/null | grep -v README @@ -79,7 +62,7 @@ ls .changeset/*.md 2>/dev/null | grep -v README - If a changeset exists, read it and verify the message is user-facing ("Add X to Y" or "Fix X in Y"), not implementation details. - Remind: use `skip changelog` label only for CI/test/docs changes, never for bug fixes. -### Step 7: Output report +### Step 5: Output report Present a pass/fail checklist: @@ -90,15 +73,10 @@ PR Preparation Report [PASS/FAIL] Lint .................. {details} [PASS/FAIL] Typecheck ............. {details} [PASS/FAIL] Tests ................. {details} -[PASS/FAIL] Stories have play ..... {details} -[PASS/FAIL] No !important ........ {details} -[PASS/FAIL] No hardcoded colors .. {details} -[PASS/FAIL] No :global ........... {details} -[PASS/FAIL] No cross-component ... {details} -[PASS/FAIL] No forwardRef ........ {details} -[PASS/FAIL] Changeset ............ {details} - -{N}/10 checks passed. +[PASS/FAIL] Rule violations ....... {details} +[PASS/FAIL] Changeset ............. {details} + +{N}/5 checks passed. ``` If all pass, the PR is ready for submission. If any fail, list the specific files and lines that need fixing. diff --git a/.cursor/skills/rca-debug/SKILL.md b/.cursor/skills/rca-debug/SKILL.md new file mode 100644 index 000000000..a1dc16029 --- /dev/null +++ b/.cursor/skills/rca-debug/SKILL.md @@ -0,0 +1,60 @@ +--- +name: rca-debug +description: Structured root cause analysis for persistent bugs. Use when a bug resists simple fixes, when the user says "debug this", "RCA", or "this keeps failing". +--- + +# Root Cause Analysis Skill + +For bugs that resist quick fixes. Do not patch symptoms -- find and fix the root cause. + +## Steps + +### Step 1: Reproduce + +Create a minimal, reliable reproduction of the failure before attempting any fix. + +1. State the **expected behavior** clearly. +2. Identify the **exact trigger** (input, sequence, timing) that causes the failure. +3. If possible, write a failing test (`*.browser.test.tsx` or unit test) that captures the bug. + +Do not proceed to Step 2 until you can reproduce the failure on demand. + +### Step 2: Hypothesize + +Formulate a single, testable hypothesis about the cause. + +Format: "Hypothesis: [specific claim about what is wrong and why]" + +Example: "Hypothesis: The combobox input value resets because Ark's `onInputValueChange` fires after our `setValue` call, overwriting it with stale state." + +### Step 3: Experiment + +Design a non-destructive observation to prove or disprove the hypothesis. + +- Add temporary logging, read state, inspect the call stack, check timing +- Do NOT apply a fix yet -- gather evidence first + +State the result: "Confirmed" or "Disproven: [what the evidence actually showed]" + +If disproven, return to Step 2 with a new hypothesis informed by the new evidence. + +### Step 4: Fix + +With a confirmed root cause, implement the minimal fix. + +- Fix the cause, not the symptom +- If the root cause is in a shared component, check all consumers +- Follow project rules (`standards.mdc`, `react-patterns.mdc`, etc.) + +### Step 5: Verify + +1. Re-run the failing reproduction from Step 1 -- it must now pass. +2. Run related tests: `pnpm --filter test --run` +3. Lint and typecheck the affected package. + +### Anti-patterns (forbidden) + +- Applying a fix without a confirmed root cause +- Re-trying a previously failed fix without new evidence +- Patching a symptom (e.g. adding a null check) without understanding why the value is null +- Skipping reproduction and jumping straight to code changes diff --git a/README.md b/README.md index 48cfd3b5c..1bdd65e43 100644 --- a/README.md +++ b/README.md @@ -80,35 +80,39 @@ This repo includes rules, skills, and notepads in `.cursor/` to support AI-power Auto-applied contextual guidance for the AI agent. -| Rule | Scope | Description | -| -------------------- | ----------------------------- | --------------------------------------- | -| `standards.mdc` | Always | PR requirements, code org, PR checklist | -| `checkers.mdc` | Always | How to run lint / test / typecheck | -| `react-patterns.mdc` | `**/*.tsx`, `**/*.ts` | Memoization, hooks, React 19, Ark UI | -| `storybook.mdc` | `**/*.stories.tsx` | Play tests, fn(), a11y, mockdate | -| `scss.mdc` | `**/*.scss` | Design tokens, no !important, mixins | -| `design-system.mdc` | `packages/**/components/**/*` | Component scaffolding templates | -| `monorepo.mdc` | `packages/**/*` | Import boundaries | -| `code-review.mdc` | Manual / on diff | Local PR review with inline comments | +| Rule | Scope | Description | +| -------------------- | --------------------------------- | ------------------------------------------------ | +| `standards.mdc` | Always | Code standards, component API design | +| `checkers.mdc` | Always | How to run lint / test / typecheck | +| `react-patterns.mdc` | `**/*.tsx`, `**/*.ts` | Hooks, memoization, React 19, Ark UI | +| `storybook.mdc` | `**/*.stories.tsx` | Story layout, args, styling, mockdate | +| `browser-tests.mdc` | `**/__tests__/*.browser.test.tsx` | Vitest browser test patterns and a11y queries | +| `scss.mdc` | `**/*.scss` | Design tokens, no !important, mixins | +| `design-system.mdc` | `packages/**/components/**/*` | Component conventions, primitive library choices | +| `monorepo.mdc` | `packages/**/*` | Import boundaries | +| `code-review.mdc` | Manual / on diff | PR workflow, checklist, inline review comments | #### Skills (`.cursor/skills/`) Multi-step workflows the AI agent executes on request. -| Skill | Trigger | What it does | -| ---------------------- | --------------------------- | ---------------------------------------------------------------- | -| **component-scaffold** | "Scaffold a new component" | Checks Ark UI, creates files, wires exports, generates stories | -| **figma-to-component** | "Implement this Figma link" | Extracts design context + tokens from Figma, scaffolds component | -| **pr-prep** | "Prepare my PR" | Runs lint/typecheck/test on diff, validates stories + SCSS | +| Skill | Trigger | What it does | +| ----------------------- | --------------------------- | ---------------------------------------------------------------- | +| **component-scaffold** | "Scaffold a new component" | Checks Ark UI, creates files, wires exports, generates stories | +| **figma-to-component** | "Implement this Figma link" | Extracts design context + tokens from Figma, scaffolds component | +| **pr-prep** | "Prepare my PR" | Runs lint/typecheck/test on diff, validates changeset | +| **migrate-story-tests** | "Migrate tests for ds-X" | Converts Storybook play functions to Vitest browser tests | +| **rca-debug** | "Debug this" / "RCA" | Structured root cause analysis for persistent bugs | +| **deslop** | "Clean up this code" | Removes AI-generated code slop, fixes style | +| **get-pr-comments** | "Get PR comments" | Fetches and summarizes review comments from the active PR | #### Notepads (`.cursor/notepads/`) Reusable prompt snippets you invoke with `@` in Cursor chat. -| Notepad | Usage | -| ------------------ | -------------------------------------------------- | -| **add-play-tests** | Generate missing `play` functions for stories | -| **check-ark-ui** | Query Ark UI for primitives before building custom | +| Notepad | Usage | +| ---------------- | -------------------------------------------------- | +| **check-ark-ui** | Query Ark UI for primitives before building custom | #### How to use @@ -125,12 +129,15 @@ The one exception is `code-review.mdc`, which you trigger manually: "Scaffold a ds-tooltip component" β†’ component-scaffold "Implement this " β†’ figma-to-component "Prepare my PR" β†’ pr-prep +"Migrate tests for ds-toggle" β†’ migrate-story-tests +"Debug this" / "RCA" β†’ rca-debug +"Clean up this code" β†’ deslop +"Get PR comments" β†’ get-pr-comments ``` **Notepads** are invoked with `@` in Cursor chat: ``` -@add-play-tests β†’ generate missing play functions @check-ark-ui β†’ check Ark UI before building custom ``` @@ -139,7 +146,7 @@ The one exception is `code-review.mdc`, which you trigger manually: ``` 1. "Scaffold a ds-card component" (or "Implement this ") β†’ agent checks Ark UI for primitives, creates all files, - wires barrel exports, generates stories with play tests + wires barrel exports, generates stories (+ optional browser tests) β†’ with a Figma URL: also extracts design tokens, maps to CSS custom properties, and pre-fills styles/variants/stories β†’ rules like react-patterns, scss, storybook, design-system @@ -148,8 +155,7 @@ The one exception is `code-review.mdc`, which you trigger manually: 2. Iterate on the component in chat β†’ rules keep guiding the agent (tokens, no !important, a11y queries, etc.) -3. @add-play-tests - β†’ backfills any missing play functions in your stories +3. Add or extend `__tests__/*.browser.test.tsx` for interactions you care about 4. "Review my changes" β†’ agent diffs against origin/main and drops inline REVIEW-* comments diff --git a/cspell.json b/cspell.json index fac311cba..1fd5cd911 100644 --- a/cspell.json +++ b/cspell.json @@ -10,6 +10,7 @@ "borderless", "clickability", "controlify", + "deslop", "drivenets", "evenodd", "findlast",