You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
[Bug Report] Nothing tests the instance expose surface — add coverage so missing AtomExpose/focus fails CI #926
surface.test.ts freezes the exported names — composables, components, utilities, public types — but nothing asserts what a component instance actually exposes to a template ref. A component can ship exposing nothing and every test still passes.
That is how #923 happened: CheckboxRoot, RadioRoot and SwitchRoot each wrap an Atom, render a focusable <button tabindex="0">, and never call defineExpose. .claude/rules/components.md:581 requires it —
Always expose AtomExpose when the sub-component wraps an Atom and an outer parent might need the underlying element (focus management, roving tabindex, pointer tracking, useResizeObserver).
— and there was no test that could notice.
Requested by @userquin as the follow-up guard to #923.
Verifying the fix belongs in that PR. This issue is the wider guard: only Atom, SplitterRoot and SplitterPanel call defineExpose anywhere in packages/0/src/components. The three roots in #923 are the ones we happened to trip over; the omission is very likely broader, and without a test it recurs on the next component added.
It also resolves a real tension in our own rules. .claude/rules/testing.md says "Only write tests when explicitly asked — never proactively add test files", while new-feature-checklist.md requires coverage. Treat this issue as the explicit ask.
Proposal
Mirror the existing cross-component pattern. components/a11y.browser.test.ts already iterates every component through components/fixtures/*.vue; an expose.browser.test.ts alongside it can walk the same fixtures and assert:
Every Root that renders through Atom exposes element, and it resolves to the real DOM node (not undefined)
Where the root element is focusable, element.focus() moves document.activeElement to it
A freeze-list of expected expose keys per component — the same shape as surface.test.ts's COMPONENTS array — would make additions deliberate: adding a component fails the test until its expose surface is declared.
The element is focusable. The ref hands back an empty object. Nothing in the suite fails.
Blocked on
#923 for the focus() assertions. Points 1 and 2 above are testable today and would fail on current master — which is arguably a reason to land them first, so #923 has a red test to turn green.
Summary
surface.test.tsfreezes the exported names — composables, components, utilities, public types — but nothing asserts what a component instance actually exposes to a template ref. A component can ship exposing nothing and every test still passes.That is how #923 happened:
CheckboxRoot,RadioRootandSwitchRooteach wrap anAtom, render a focusable<button tabindex="0">, and never calldefineExpose..claude/rules/components.md:581requires it —— and there was no test that could notice.
Requested by @userquin as the follow-up guard to #923.
Why this isn't just "add tests to the #923 PR"
Verifying the fix belongs in that PR. This issue is the wider guard: only
Atom,SplitterRootandSplitterPanelcalldefineExposeanywhere inpackages/0/src/components. The three roots in #923 are the ones we happened to trip over; the omission is very likely broader, and without a test it recurs on the next component added.It also resolves a real tension in our own rules.
.claude/rules/testing.mdsays "Only write tests when explicitly asked — never proactively add test files", whilenew-feature-checklist.mdrequires coverage. Treat this issue as the explicit ask.Proposal
Mirror the existing cross-component pattern.
components/a11y.browser.test.tsalready iterates every component throughcomponents/fixtures/*.vue; anexpose.browser.test.tsalongside it can walk the same fixtures and assert:Atomexposeselement, and it resolves to the real DOM node (notundefined)element.focus()movesdocument.activeElementto itfocus(options?: FocusOptions)focuses the selected item, falling back to the firstA freeze-list of expected expose keys per component — the same shape as
surface.test.ts'sCOMPONENTSarray — would make additions deliberate: adding a component fails the test until its expose surface is declared.Measured today (master)
The element is focusable. The ref hands back an empty object. Nothing in the suite fails.
Blocked on
#923 for the
focus()assertions. Points 1 and 2 above are testable today and would fail on currentmaster— which is arguably a reason to land them first, so #923 has a red test to turn green.