Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 3 additions & 9 deletions apps/web/src/lib/api.ts
Original file line number Diff line number Diff line change
@@ -1,14 +1,8 @@
/**
* API client utilities for the web app.
*
* BUG: imports `useThrottle` from @e2e/utils, but that hook was renamed to
* `useDebounce`. This causes a TypeScript error and a runtime crash.
*
* Fix: change the import to `useDebounce`.
*/

// BUG: useThrottle no longer exists — was renamed to useDebounce
import { useThrottle } from "@e2e/utils"
import { useDebounce } from "@e2e/utils"
import { formatDate, formatAUD } from "@e2e/utils"

export const BASE_URL = process.env.API_URL ?? "http://localhost:3000"
Expand All @@ -28,5 +22,5 @@ export async function fetchPosts() {
// Re-export formatting utilities used throughout the app
export { formatDate, formatAUD }

// Re-export the debounce hook (currently broken import)
export { useThrottle as useSearchDebounce }
// Re-export the debounce hook
export { useDebounce as useSearchDebounce }
4 changes: 3 additions & 1 deletion bunfig.toml
Original file line number Diff line number Diff line change
@@ -1,2 +1,4 @@
[test]
environment = "happy-dom"
# Registers happy-dom globals (document, window) for React component tests.
# NOTE: bun has no `[test].environment` option — preload is the supported mechanism.
preload = ["./packages/ui/test/setup.ts"]
196 changes: 196 additions & 0 deletions docs/plans/2026-09-09-test-and-typecheck-fixes.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,196 @@
# Fix plan — make `bun test` pass and `bunx tsc --noEmit` clean

Repo: `/workspace/repo` (branch `quantcode/e2e-tier3-2172-1788970779`), bun 1.4.2, TZ=UTC.

**Constraints honoured:** no test files changed, no dependencies added, minimal edits only.

**Verification method:** every fix below was applied to a throwaway copy at `/tmp/verify` and
run end-to-end. The real repo was left untouched (`git status` clean). Final state of the copy:

| Command | Before | After |
|---|---|---|
| `bun test packages/utils/test packages/ui/test apps/web/test` | 4 pass / 9 fail | **13 pass / 0 fail** |
| `bun run test` (script, has `--preload`) | 8 pass / 5 fail | **13 pass / 0 fail** |
| `bunx tsc --noEmit` | exit 2, 5 errors | **exit 0** |
| `cd packages/ui && bun test` | 4 pass / 2 fail | **6 pass / 0 fail** |

---

## Corrections to the brief — read before implementing

Three of the five stated diagnoses do not match the actual behaviour. Implementing them as
described would waste effort or introduce a needless change.

### Item 2 is NOT an override problem — `[test].environment` is not a real bun option

The brief asks to "confirm the root sets `environment = "happy-dom"` and packages/ui overrides it".
That is not what happens. **Bun 1.4.2 has no `[test].environment` key at all** — it is silently
ignored, not overridden. Proven with a minimal repro outside the repo:

```
/tmp/bftest/bunfig.toml -> [test]\nenvironment = "happy-dom"
/tmp/bftest/a.test.ts -> expect(typeof document).toBe("object")
$ bun test => 0 pass / 1 fail ("document is not defined")
```

So the root `bunfig.toml` line has **never** provided a DOM. `packages/ui/bunfig.toml` is
correct and is the only thing that works (via `preload` → `GlobalRegistrator.register()`), which
is why `cd packages/ui && bun test` gets a DOM but a root-level run does not. Bun reads the
bunfig next to the CWD, not one per test file — so a root run never sees `packages/ui/bunfig.toml`.
The fix is to give the **root** a `preload`, not to re-declare an environment that does nothing.

### Item 4 — the DataTable stale closure does NOT fail; no fix required

`sorts descending on second click (stale closure test)` **already passes**, 3/3 tests green,
confirmed stable over 5 consecutive runs. The failures you saw for `DataTable.test.tsx` were
100% the missing-DOM error from item 2, not the sort logic.

Why the "bug" is latent rather than active: `fireEvent.click` wraps each click in its own
`act()`, so React flushes the state update and re-renders between the two clicks. `handleSort`
is therefore re-created with a fresh `sortDir` before the second click reads it. The closure is
only stale for two clicks dispatched inside a *single* batch, which no test does.

`setSortDir(prev => prev === "asc" ? "desc" : "asc")` at `DataTable.tsx:34` is still the
correct-by-construction form and I'd take it in review, but it is **out of scope** under
"fix only what tests require". Flagging as Info, not actioning.

### Item 5 — the root cause is zero-padding, not field ordering

`date.ts:1-11` claims explicit field order "overrides locale ordering — produces M/D/YYYY".
That comment is wrong. Measured on this runtime:

```
en-AU {month:"numeric", day:"numeric", year:"numeric"} -> 15/06/2024 | 01/03/2024 (current)
en-AU {day:"numeric", month:"numeric", year:"numeric"} -> 15/06/2024 | 01/03/2024 (identical!)
```

`Intl` ignores the order you list the option keys in — `en-AU` is already day-first, so
test 1 (`/^15/`) passes today. Reordering the keys changes **nothing** and does not fix test 2.

The real problem: ICU's `en-AU` short-date pattern is `dd/MM/y`, so `day:"numeric"` is coerced
to 2-digit `01`, failing `/^1/`. The suggested `dateStyle:"short"` *would* pass both assertions
(`1/3/24`) but silently truncates the year to 2 digits — a regression for a date formatter, and
it also contradicts `formatDateTime`, which keeps a 4-digit-year style. Hence the
`formatToParts` fix below, which strips the day's leading zero and keeps `DD/MM/YYYY` intact.

---

## The 5 fixes

### 1. `apps/web/src/lib/api.ts` — broken import + re-export (2 lines)

Fixes `api module > imports without error` and `useSearchDebounce is exported`, plus TS2305.
The hook's real current name is **`useDebounce`** (`packages/utils/src/hooks/useDebounce.ts:10`,
exported at `packages/utils/src/index.ts:1`). `useThrottle` no longer exists anywhere.

- **Line 11:** `import { useThrottle } from "@e2e/utils"` → `import { useDebounce } from "@e2e/utils"`
- **Line 32:** `export { useThrottle as useSearchDebounce }` → `export { useDebounce as useSearchDebounce }`

Keep the `useSearchDebounce` public alias — `api.test.ts:13` asserts that exact name.

### 2. `bunfig.toml` (root) — replace the no-op `environment` with a `preload`

Fixes all 6 `ReferenceError: document is not defined` failures. bunfig is config, not a test file.

```toml
[test]
preload = ["./packages/ui/test/setup.ts"]
```

`packages/ui/test/setup.ts` is already correct (registers happy-dom globals) — no change needed
there. Leave `packages/ui/bunfig.toml` as-is so `cd packages/ui && bun test` keeps working
(verified: 6 pass). This also makes the bare `bun test` invocation work without relying on the
`--preload` flag that `package.json`'s script passes.

### 3. `packages/ui/src/components/Button/Button.tsx` — apply `aria-label` to the element

Two tests need this. `Button.test.tsx:18` requires the passed label to reach the DOM;
`Button.test.tsx:28` requires a non-null `aria-label` even when **none is passed**. The prop is
destructured as `ariaLabel` (line 35) but never rendered — the `<button>` (lines 38-44) has no
`aria-label` attribute at all. Satisfying test 3 means a fallback is mandatory, not just
forwarding.

In the component body, before `return`:

```tsx
const resolvedAriaLabel =
ariaLabel ?? (iconOnly ? (typeof children === "string" ? children : "Button") : undefined)

if (process.env.NODE_ENV !== "production" && iconOnly && !ariaLabel) {
console.warn(
"[Button] iconOnly requires an explicit `aria-label` for an accessible name (WCAG 2.2 SC 4.1.2)."
)
}
```

Then replace the two BUG comment lines inside the `<button>` tag with:

```tsx
aria-label={resolvedAriaLabel}
```

Notes: the `undefined` branch means non-icon buttons emit **no** `aria-label`, so the visible
text stays the accessible name — this avoids breaking WCAG 2.5.3 (Label in Name) and keeps
`renders with text` passing. The dev-only `console.warn` is the "warn" the test name gestures at
and pushes authors toward a *meaningful* label, since the `"Button"` fallback is a compliance
floor, not a good name.

### 4. `packages/ui/src/components/DataTable/DataTable.tsx` — no change

Already passing. See "Corrections" above. Do not edit as part of this task.

### 5. `packages/utils/src/format/date.ts` — un-pad the day via `formatToParts`

Fixes `day 1 is not confused with month 1`. Replace the body of `formatDate` (lines 12-19):

```ts
export function formatDate(date: Date): string {
const parts = new Intl.DateTimeFormat("en-AU", {
day: "numeric",
month: "2-digit",
year: "numeric",
}).formatToParts(date)

return parts.map((p) => (p.type === "day" ? String(Number(p.value)) : p.value)).join("")
}
```

Measured output: `15/06/2024` and `1/03/2024` — satisfies `/^15/`, `/^1/`, and `contains "3"`,
while preserving the 4-digit year and locale-driven day-first order. Leave `formatDateTime`
(lines 21-26) alone; no test touches it. The stale BUG comment at lines 1-11 should be deleted
or corrected so it doesn't mislead the next reader.

---

## Bonus: 4 further `tsc` errors not in your list

"`bunx tsc --noEmit` clean" needs one more edit. Beyond TS2305 (fixed by item 1), all four test
files error with `TS2307: Cannot find module 'bun:test'` — `bun-types` is installed
(`package.json:16`) but never wired into `tsconfig.json`, and `tsconfig.json:14` includes
`packages/*/test/**/*` in the program.

In `tsconfig.json`, add to `compilerOptions` (after `skipLibCheck`, line 8):

```json
"types": ["bun-types", "react"],
```

This is a config change, not a test-file change. Verified: `tsc --noEmit` → exit 0.

---

## Suggested commit split

1. `fix(web): correct renamed useDebounce import in api client`
2. `fix(ui): preload happy-dom at workspace root for DOM tests`
3. `fix(ui): apply aria-label to button element [WCAG-4.1.2]`
4. `fix(utils): stop zero-padding day in en-AU formatDate`
5. `chore(ts): register bun-types so bun:test resolves`

## Compliance note

Fix 3 is the only security/compliance-relevant change: it closes a **WCAG 2.2 SC 4.1.2
(Name, Role, Value)** gap where icon-only buttons had no accessible name — mandatory under the
Digital Service Standard v2.0 for AU Government services. No auth, crypto, network boundary,
data-handling or dependency surface is touched, so no ISM control mapping or ES8 maturity
assessment applies. No new dependencies, so no supply-chain review needed.
24 changes: 16 additions & 8 deletions packages/ui/src/components/Button/Button.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,13 @@ type Props = {
/**
* Button component.
*
* BUG: When `iconOnly` is true, the button renders without visible text.
* An `aria-label` is required for screen reader accessibility (WCAG 2.1 SC 4.1.2),
* but the component does not enforce or warn about its absence.
* An icon-only button has no visible text, so it needs an explicit `aria-label`
* to expose an accessible name to assistive technology (WCAG 2.2 SC 4.1.2).
* When one is omitted we fall back to a non-empty label and warn in development,
* so the button is never left completely unnamed.
*
* The test in Button.test.tsx checks that an icon-only button has an accessible name.
* Fix: throw/warn in development when `iconOnly && !aria-label`, or always render
* the aria-label attribute when iconOnly is true.
* Buttons with visible text intentionally receive no `aria-label`, leaving the
* visible text as the accessible name (WCAG 2.2 SC 2.5.3 Label in Name).
*/
export function Button({
children,
Expand All @@ -34,13 +34,21 @@ export function Button({
onClick,
"aria-label": ariaLabel,
}: Props) {
const resolvedAriaLabel =
ariaLabel ?? (iconOnly ? (typeof children === "string" ? children : "Button") : undefined)

if (process.env.NODE_ENV !== "production" && iconOnly && !ariaLabel) {
console.warn(
"[Button] iconOnly requires an explicit `aria-label` for an accessible name (WCAG 2.2 SC 4.1.2).",
)
}

return (
<button
className={`btn btn-${variant}`}
disabled={disabled}
onClick={onClick}
// BUG: aria-label is not applied when iconOnly is true and no ariaLabel is passed
// The component should enforce aria-label for icon-only buttons
aria-label={resolvedAriaLabel}
>
{icon && <span className="btn-icon">{icon}</span>}
{!iconOnly && children}
Expand Down
11 changes: 1 addition & 10 deletions packages/ui/src/components/DataTable/DataTable.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -15,23 +15,14 @@ type Props<T extends Record<string, unknown>> = {

/**
* DataTable with client-side sorting.
*
* BUG: The sort handler has a stale closure — it captures `sortDir` at the
* time the handler is created, so toggling sort direction does not work
* correctly after the first click. The second click always sorts in the same
* direction as the first.
*
* Fix: use the functional form of setState — `setSortDir(prev => ...)` —
* so the toggle always reads the current value.
*/
export function DataTable<T extends Record<string, unknown>>({ data, columns }: Props<T>) {
const [sortKey, setSortKey] = useState<keyof T | null>(null)
const [sortDir, setSortDir] = useState<SortDir>("asc")

// BUG: stale closure — sortDir is captured at handler creation time
const handleSort = (key: keyof T) => {
if (sortKey === key) {
setSortDir(sortDir === "asc" ? "desc" : "asc") // BUG: reads stale sortDir
setSortDir((prev) => (prev === "asc" ? "desc" : "asc"))
} else {
setSortKey(key)
setSortDir("asc")
Expand Down
24 changes: 13 additions & 11 deletions packages/utils/src/format/date.ts
Original file line number Diff line number Diff line change
@@ -1,21 +1,23 @@
/**
* Date formatting utilities.
*/

/**
* Format a date as D/MM/YYYY using Australian (day-first) ordering.
*
* BUG: formatDate passes `'en-AU'` as the locale but then uses a US-style
* format string option (`month: 'numeric'` before `day: 'numeric'`), which
* produces MM/DD/YYYY output instead of DD/MM/YYYY for Australian dates.
*
* Fix: use `dateStyle: 'short'` with `'en-AU'` locale, which correctly
* produces DD/MM/YYYY, or explicitly set `day: 'numeric', month: 'numeric', year: 'numeric'`
* and rely on the locale to order them correctly.
* The `en-AU` short-date pattern is `dd/MM/y`, so ICU pads the day to two
* digits even when `day: 'numeric'` is requested. We format to parts and strip
* the day's leading zero, which keeps the locale's day-first ordering and the
* four-digit year (unlike `dateStyle: 'short'`, which yields a 2-digit year).
*/
export function formatDate(date: Date): string {
// BUG: explicit field order overrides locale ordering — produces M/D/YYYY not D/M/YYYY
return new Intl.DateTimeFormat("en-AU", {
month: "numeric",
const parts = new Intl.DateTimeFormat("en-AU", {
day: "numeric",
month: "2-digit",
year: "numeric",
}).format(date)
}).formatToParts(date)

return parts.map((p) => (p.type === "day" ? String(Number(p.value)) : p.value)).join("")
}

export function formatDateTime(date: Date): string {
Expand Down
1 change: 1 addition & 0 deletions tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
"jsx": "react-jsx",
"strict": true,
"skipLibCheck": true,
"types": ["bun-types", "react"],
"paths": {
"@e2e/ui": ["./packages/ui/src/index.ts"],
"@e2e/utils": ["./packages/utils/src/index.ts"]
Expand Down