docs(plans): bundle vendor-split + test gaps — 3-task plan (#67)

2026-06-09 12:16:23 +02:00
parent 79ee402b33
commit 0b44bc0855
1 changed files with 338 additions and 0 deletions
@@ -0,0 +1,338 @@
+# Bundle Vendor-Split + Test-Gap Fills — Implementation Plan
+
+> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking.
+
+**Goal:** Split framework deps into cache-stable vendor chunks, and fill the audit's unit-test (prune-fields, delete-confirm-dialog in-use, labels, format-date) + combobox-story gaps.
+
+**Architecture:** Task 1 adds `build.rollupOptions.output.manualChunks` to `vite.config.ts` (production build only). Task 2 adds 4 jsdom unit tests for pure/critical units. Task 3 adds a Storybook story for the combobox primitive and runs the full gate. All additive/behavior-preserving.
+
+**Tech Stack:** Vite 6 (Rollup), React 19 + TS + pnpm, Vitest 4 (jsdom + storybook-browser projects), Storybook, RTL + MSW.
+
+**Conventions:** pnpm; **no `any`/`eslint-disable`/`@ts-ignore`**; no codename; app/test source double-quote+semicolon; **stories use single-quote + no-semicolon** (mirror existing `ui/*.stories.tsx`). Run a single test pass per task.
+
+**Spec:** `docs/superpowers/specs/2026-06-09-bundle-and-tests-design.md`
+
+**Key facts:**
+- `scripts/check-bundle-size.mjs` fails if the **largest** `dist/assets/*.js` gzip exceeds 250 KB; today ~216.5 KB (one chunk).
+- `vite.config.ts` exports `defineConfig({ plugins, resolve, server, test })` — no `build` block. Deps: `react`, `react-dom`, `react-router-dom` (v7, re-exports `react-router`), `@tanstack/react-query`, `@base-ui/react`, `i18next`, `react-i18next`.
+- `objects/prune-fields.ts`: `pruneFields(fields: Record<string, unknown>, localizedTextKeys: Set<string>, defaultLang: string): Record<string, unknown>`.
+- `lib/labels.ts`: `labelText(labels: LabelView[], lang: string): string` (lang match → `"en"` → first → `""`).
+- `lib/format-date.ts`: `formatDate(value: unknown, lang: string): string` (non-string null→`"—"`, non-string→`String(value)`; invalid→returns value; valid `YYYY-MM-DD`→`Intl.DateTimeFormat(lang,{dateStyle:"medium"})`, parsed at local midnight).
+- `components/delete-confirm-dialog.tsx`: `DeleteConfirmDialog({ description, onConfirm: () => Promise<void>, triggerLabel? })`. Trigger is a `Button` labelled `t("actions.delete")` ("Delete"); the confirm action is also labelled `t("actions.delete")`. On a thrown error it sets a message via `errorMessageKey(err)` and **returns without closing**; on success it closes. `actions.inUse` (en) = "Can't delete — used by {{count}} object(s). Clear those fields first." `InUseError` is exported from `../api/errors` (and re-exported by `../api/queries`).
+- `components/ui/combobox.tsx` exports `ComboboxRoot<Value>`, `ComboboxInputGroup`, `ComboboxInput`, `ComboboxClear`, `ComboboxTrigger`, `ComboboxPopup`, `ComboboxList`, `ComboboxItem`, `ComboboxItemIndicator`, `ComboboxEmpty`. See `objects/options-combobox.tsx` for the composition (ComboboxRoot props: `items`, `value`, `onValueChange`, `itemToStringLabel`, `isItemEqualToValue`; ComboboxList takes a render-function child).
+- Story format (from `components/ui/badge.stories.tsx`): `import type { Meta, StoryObj } from '@storybook/react-vite'`; `import { expect } from 'storybook/test'`; `const meta = { component, tags: ['ai-generated'] } satisfies Meta<typeof X>`; `export default meta`; `type Story = StoryObj<typeof meta>`; `export const Default: Story = { play: async ({ canvas }) => {...} }`.
+
+---
+
+# Task 1: Vendor split (`vite.config.ts`)
+
+**Files:** Modify `web/vite.config.ts`.
+
+- [ ] **Step 1: Add the `build` block.** In `web/vite.config.ts`, add a top-level `build` key to the `defineConfig({...})` object (sibling of `plugins`/`resolve`/`server`/`test`):
+```ts
+  build: {
+    rollupOptions: {
+      output: {
+        manualChunks: {
+          react: ["react", "react-dom", "react-router", "react-router-dom"],
+          "base-ui": ["@base-ui/react"],
+          query: ["@tanstack/react-query"],
+          i18n: ["i18next", "react-i18next"],
+        },
+      },
+    },
+  },
+```
+
+- [ ] **Step 2: Build + verify chunk split + budget:**
+```bash
+cd web && pnpm build && pnpm check:size && ls -1 dist/assets/*.js
+```
+Expected: build succeeds; `check:size` prints `largest JS chunk: <name> = <kb> KB gz (budget 250 KB)` and passes; `dist/assets/` now contains separate `react-*.js`, `base-ui-*.js`, `query-*.js`, `i18n-*.js` chunks plus the app entry. Report the largest chunk size (should be a vendor chunk, well under 250). If the build warns about React being in multiple chunks or a chunk is oversized, confirm `react`+`react-dom`+`react-router`+`react-router-dom` are all in the one `react` group and adjust.
+
+- [ ] **Step 3: typecheck + lint** (config change shouldn't affect them, but confirm):
+```bash
+cd web && pnpm typecheck && pnpm lint
+```
+
+- [ ] **Step 4: Commit**
+```bash
+cd /Users/olsson/Laboratory/biggus-dickus
+git add web/vite.config.ts
+git commit -m "build(web): split framework deps into cache-stable vendor chunks (#67)"
+```
+
+---
+
+# Task 2: Unit-test gaps (prune-fields, labels, format-date, delete-confirm-dialog)
+
+**Files:** Create `web/src/objects/prune-fields.test.ts`, `web/src/lib/labels.test.ts`, `web/src/lib/format-date.test.ts`, `web/src/components/delete-confirm-dialog.test.tsx`.
+
+- [ ] **Step 1: `web/src/objects/prune-fields.test.ts`:**
+```ts
+import { expect, test } from "vitest";
+
+import { pruneFields } from "./prune-fields";
+
+test("drops empty/null/undefined scalars, keeps real scalars", () => {
+  const out = pruneFields(
+    { a: "x", b: "", c: null, d: undefined, e: 0, f: false },
+    new Set(),
+    "en",
+  );
+  expect(out).toEqual({ a: "x", e: 0, f: false });
+});
+
+test("a localized_text key keeps only the default-language entry", () => {
+  const out = pruneFields(
+    { title: { en: "Bowl", sv: "Skål" } },
+    new Set(["title"]),
+    "sv",
+  );
+  expect(out).toEqual({ title: { sv: "Skål" } });
+});
+
+test("a non-localized object value keeps all non-empty entries", () => {
+  const out = pruneFields(
+    { dims: { w: "10", h: "", d: "5" } },
+    new Set(),
+    "en",
+  );
+  expect(out).toEqual({ dims: { w: "10", d: "5" } });
+});
+
+test("an object value left with no entries is dropped entirely", () => {
+  const out = pruneFields(
+    { title: { en: "Bowl" }, empty: { en: "", sv: "" } },
+    new Set(["title", "empty"]),
+    "sv",
+  );
+  // title has no `sv` entry → empty after filtering → dropped; empty → dropped
+  expect(out).toEqual({});
+});
+```
+Run: `cd web && pnpm vitest run src/objects/prune-fields.test.ts`. (If a case's expectation mismatches the real semantics, re-read `prune-fields.ts` and correct the EXPECTATION to the true behavior — do not change the source.)
+
+- [ ] **Step 2: `web/src/lib/labels.test.ts`:**
+```ts
+import { expect, test } from "vitest";
+
+import { labelText } from "./labels";
+
+const labels = [
+  { lang: "en", label: "Bowl" },
+  { lang: "sv", label: "Skål" },
+];
+
+test("returns the exact-language label when present", () => {
+  expect(labelText(labels, "sv")).toBe("Skål");
+});
+
+test("falls back to the English label when the requested language is missing", () => {
+  expect(labelText(labels, "de")).toBe("Bowl");
+});
+
+test("falls back to the first label when neither the language nor English is present", () => {
+  expect(labelText([{ lang: "fr", label: "Bol" }], "de")).toBe("Bol");
+});
+
+test("returns an empty string for no labels", () => {
+  expect(labelText([], "en")).toBe("");
+});
+```
+Run: `cd web && pnpm vitest run src/lib/labels.test.ts`.
+
+- [ ] **Step 3: `web/src/lib/format-date.test.ts`:**
+```ts
+import { expect, test } from "vitest";
+
+import { formatDate } from "./format-date";
+
+test("formats a date-only string in the locale without a timezone day-shift", () => {
+  // local-midnight parse must keep the calendar day (3), not shift to the 2nd
+  expect(formatDate("1962-04-03", "en")).toBe("Apr 3, 1962");
+});
+
+test("returns the em-dash placeholder for null", () => {
+  expect(formatDate(null, "en")).toBe("—");
+});
+
+test("returns an unparseable string unchanged", () => {
+  expect(formatDate("not-a-date", "en")).toBe("not-a-date");
+});
+
+test("stringifies a non-string, non-null value", () => {
+  expect(formatDate(42, "en")).toBe("42");
+});
+```
+Run: `cd web && pnpm vitest run src/lib/format-date.test.ts`. (The CI/runtime is full-ICU Node 22, so en `dateStyle:"medium"` → `"Apr 3, 1962"`. If your local ICU formats slightly differently, assert the equivalent string but keep the day as `3` — the day-shift guard is the point.)
+
+- [ ] **Step 4: `web/src/components/delete-confirm-dialog.test.tsx`:**
+```tsx
+import { expect, test, vi } from "vitest";
+import { screen, waitFor, within } from "@testing-library/react";
+import userEvent from "@testing-library/user-event";
+
+import { renderApp } from "../test/render";
+import { DeleteConfirmDialog } from "./delete-confirm-dialog";
+import { InUseError } from "../api/errors";
+
+test("delete-in-use shows the in-use count and keeps the dialog open", async () => {
+  const onConfirm = vi.fn<() => Promise<void>>().mockRejectedValue(new InUseError(3));
+  renderApp(<DeleteConfirmDialog description="Delete this term?" onConfirm={onConfirm} />);
+
+  await userEvent.click(screen.getByRole("button", { name: /delete/i }));
+
+  const dialog = within(document.body);
+  const buttons = await dialog.findAllByRole("button", { name: /delete/i });
+  await userEvent.click(buttons[buttons.length - 1]); // the confirm action
+
+  expect(await dialog.findByText(/used by 3/i)).toBeInTheDocument();
+  // dialog stays open: its description is still shown
+  expect(dialog.getByText("Delete this term?")).toBeInTheDocument();
+});
+
+test("a clean confirm closes the dialog", async () => {
+  const onConfirm = vi.fn<() => Promise<void>>().mockResolvedValue(undefined);
+  renderApp(<DeleteConfirmDialog description="Delete this term?" onConfirm={onConfirm} />);
+
+  await userEvent.click(screen.getByRole("button", { name: /delete/i }));
+
+  const dialog = within(document.body);
+  const buttons = await dialog.findAllByRole("button", { name: /delete/i });
+  await userEvent.click(buttons[buttons.length - 1]);
+
+  await waitFor(() => expect(dialog.queryByText("Delete this term?")).toBeNull());
+  expect(onConfirm).toHaveBeenCalledTimes(1);
+});
+```
+Run: `cd web && pnpm vitest run src/components/delete-confirm-dialog.test.tsx`. (Before opening, only the trigger "Delete" button exists. After opening, the portal adds the action "Delete"; `findAllByRole` returns both, click the last. `actions.inUse` en contains "used by 3". If `vi.fn<() => Promise<void>>()` generic syntax trips the TS/vitest version, use `vi.fn(() => Promise.reject(new InUseError(3)))` / `vi.fn(() => Promise.resolve())`.)
+
+- [ ] **Step 5: Verify all four (vitest ONCE) + typecheck + lint:**
+```bash
+cd web && pnpm vitest run src/objects/prune-fields.test.ts src/lib/labels.test.ts src/lib/format-date.test.ts src/components/delete-confirm-dialog.test.tsx && pnpm typecheck && pnpm lint
+```
+All green.
+
+- [ ] **Step 6: Commit**
+```bash
+cd /Users/olsson/Laboratory/biggus-dickus
+git add web/src/objects/prune-fields.test.ts web/src/lib/labels.test.ts web/src/lib/format-date.test.ts web/src/components/delete-confirm-dialog.test.tsx
+git commit -m "test(web): cover prune-fields, labels, format-date, delete-in-use dialog (#67)"
+```
+
+---
+
+# Task 3: Combobox story + full gate
+
+**Files:** Create `web/src/components/ui/combobox.stories.tsx`.
+
+- [ ] **Step 1: Read `web/src/objects/options-combobox.tsx`** to mirror the exact `ComboboxRoot` composition (generic, props, the `ComboboxList` render-function child), then create `web/src/components/ui/combobox.stories.tsx` (single-quote + no-semicolon, matching `badge.stories.tsx`):
+```tsx
+import type { Meta, StoryObj } from '@storybook/react-vite'
+import { useState } from 'react'
+import { expect } from 'storybook/test'
+
+import {
+  ComboboxRoot,
+  ComboboxInputGroup,
+  ComboboxInput,
+  ComboboxClear,
+  ComboboxTrigger,
+  ComboboxPopup,
+  ComboboxList,
+  ComboboxItem,
+  ComboboxItemIndicator,
+  ComboboxEmpty,
+} from './combobox'
+
+const fruits = ['Apple', 'Apricot', 'Banana', 'Cherry']
+
+function ComboboxDemo() {
+  const [value, setValue] = useState<string | null>(null)
+
+  return (
+    <ComboboxRoot<string | null>
+      items={fruits}
+      value={value}
+      onValueChange={setValue}
+      itemToStringLabel={(item) => item ?? ''}
+      isItemEqualToValue={(a, b) => a === b}
+    >
+      <ComboboxInputGroup>
+        <ComboboxInput placeholder='Pick a fruit' />
+        <ComboboxClear aria-label='Clear' />
+        <ComboboxTrigger aria-label='Open' />
+      </ComboboxInputGroup>
+      <ComboboxPopup>
+        <ComboboxEmpty>No matches</ComboboxEmpty>
+        <ComboboxList>
+          {(item: string) => (
+            <ComboboxItem key={item} value={item}>
+              <ComboboxItemIndicator className='text-primary'>✓</ComboboxItemIndicator>
+              {item}
+            </ComboboxItem>
+          )}
+        </ComboboxList>
+      </ComboboxPopup>
+    </ComboboxRoot>
+  )
+}
+
+const meta = {
+  component: ComboboxDemo,
+  tags: ['ai-generated'],
+} satisfies Meta<typeof ComboboxDemo>
+
+export default meta
+type Story = StoryObj<typeof meta>
+
+export const Default: Story = {
+  play: async ({ canvas }) => {
+    await expect(canvas.getByPlaceholderText('Pick a fruit')).toBeInTheDocument()
+  },
+}
+```
+(Adjust the `ComboboxRoot` generic/props to match `options-combobox.tsx` exactly if they differ — the goal is a rendering `Default` story; keep the `play` minimal to stay stable in the browser project.)
+
+- [ ] **Step 2: Run the storybook project for this story** (browser test):
+```bash
+cd web && pnpm vitest run src/components/ui/combobox.stories.tsx
+```
+Expected: the `Default` story passes (input renders). If the Base UI combobox API needs a different prop shape, fix the story (mirror `options-combobox.tsx`); do not weaken the assertion below a render check.
+
+- [ ] **Step 3: FULL FRONTEND GATE (run tests EXACTLY ONCE):**
+```bash
+cd web && pnpm typecheck && pnpm lint && pnpm test && pnpm build && pnpm check:size && pnpm check:colors
+```
+All green. Report total test count (should be prior 268 + 4 unit + 1 story ≈ 273), the largest chunk (gz) from check:size (a vendor chunk, < 250), and the check:colors line.
+
+- [ ] **Step 4: Codename + status:**
+```bash
+cd /Users/olsson/Laboratory/biggus-dickus
+git grep -in 'biggus\|dickus' -- web/src; echo "codename-exit=$?"
+git status --short
+```
+Expected: no matches (`codename-exit=1`).
+
+- [ ] **Step 5: Commit**
+```bash
+cd /Users/olsson/Laboratory/biggus-dickus
+git add web/src/components/ui/combobox.stories.tsx
+git commit -m "test(web): add a Storybook story for the combobox primitive (#67)"
+```
+
+---
+
+## Self-Review (completed)
+
+**Spec coverage:** AC1 vendor split + verify (T1); AC2 the 4 unit tests (T2 S1-S4); AC3 combobox story (T3 S1-S2); AC4 full gate + codename (T3 S3-S4). ✓
+
+**Placeholder scan:** every test/story is complete code with concrete assertions; manualChunks is exact; the format-date ICU note and the vi.fn generic note give concrete fallbacks, not vague TODOs. ✓
+
+**Type/consistency:** `pruneFields(fields, Set, lang)`, `labelText(labels, lang)`, `formatDate(value, lang)`, `DeleteConfirmDialog({description,onConfirm})`, `InUseError(count)` from `../api/errors` — all match the spec's documented signatures; the combobox story imports the exact exports from `./combobox`. ✓
+
+## Notes
+- No new dependency, no new i18n keys. `build.rollupOptions` only affects `pnpm build`; the test projects are unaffected.
+- The combobox story is the one item with a (small) CI cost (a browser test on the slow runner); it's isolated in its own task/commit so it can be dropped cleanly if undesired.
+- `check:size` should report a smaller largest chunk after the split (a vendor chunk, not the combined app+vendor chunk).