From 9c671aca473e2b6e38ea03f2cd74177074fa750d Mon Sep 17 00:00:00 2001 From: Matt Silverlock Date: Sat, 5 Sep 2026 03:33:13 -0400 Subject: [PATCH 1/4] Fix component value inference and public API contracts --- .gitignore | 2 + docs/reviews/typescript-component-api.md | 72 +++++++++++++++ docs/src/high-value-components.tsx | 53 +++++++++-- docs/src/main.tsx | 16 +++- docs/src/next-components.tsx | 7 +- package.json | 2 +- src/components/accordion.tsx | 6 +- src/components/alert-dialog.tsx | 2 +- src/components/autocomplete.tsx | 17 +++- src/components/dialog.tsx | 5 +- src/components/empty.tsx | 7 +- src/components/popover.tsx | 5 +- src/components/radio-group.tsx | 5 +- src/components/select.tsx | 14 +-- src/components/slider.tsx | 6 +- src/components/switch.tsx | 7 +- src/components/toggle-group.tsx | 6 +- tests/browser/component-api.spec.ts | 45 ++++++++++ tests/component-api.test.tsx | 66 ++++++++++++++ tests/component-api.types.tsx | 110 +++++++++++++++++++++++ tests/components.test.tsx | 4 +- tsconfig.json | 6 +- tsconfig.package.json | 8 ++ 23 files changed, 430 insertions(+), 41 deletions(-) create mode 100644 docs/reviews/typescript-component-api.md create mode 100644 tests/browser/component-api.spec.ts create mode 100644 tests/component-api.test.tsx create mode 100644 tests/component-api.types.tsx create mode 100644 tsconfig.package.json diff --git a/.gitignore b/.gitignore index 3bbd454..414da38 100644 --- a/.gitignore +++ b/.gitignore @@ -5,3 +5,5 @@ docs/dist/ .playwright-mcp/ .DS_Store *.tsbuildinfo +test-results/ +playwright-report/ diff --git a/docs/reviews/typescript-component-api.md b/docs/reviews/typescript-component-api.md new file mode 100644 index 0000000..65f82bf --- /dev/null +++ b/docs/reviews/typescript-component-api.md @@ -0,0 +1,72 @@ +# TypeScript and component API review + +Reviewed all 43 component modules, the public barrel, declaration build, package exports, +fieldset context, store shims, docs examples, and validation configuration against +`c56271ce216bbd3d7603359869ad1c23ee127030` (0.6.2). The compiler already enables `strict`, +`noUncheckedIndexedAccess`, and `exactOptionalPropertyTypes`; the actionable type defects +were in wrapper contracts and consumer coverage. + +## Scope + +| Area | Modules inspected | +| ------------------------ | --------------------------------------------------------------------------------------------------------------------- | +| Form controls | checkbox, checkbox-group, date-picker, field, fieldset, input, input-group, number-field, radio-group, select, switch | +| Selection and disclosure | accordion, autocomplete, collapsible, combobox, slider, tabs, toggle-button, toggle-group | +| Overlays | alert-dialog, context-menu, dialog, layer, menu, popover, toast, tooltip | +| Layout and composition | breadcrumbs, button, group-box, scroll-area, separator, table, toolbar, window | +| Status and content | badge, banner, empty, loader, meter, pagination, progress, segmented-meter | + +The review examined generic inference, inherited native props, refs, slot forwarding, +controlled and uncontrolled state, disabled-state propagation, accessible names, portal +ownership, emitted declarations, and package consumption. Source inspection covered every +module; focused regressions target the confirmed findings below. + +## Validated findings and fixes + +| Finding | Evidence before the fix | Change and regression coverage | +| ------------------------------------------------------------------------ | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | +| Select erases value and multiplicity types | String-only options still accept numeric values and single/array mismatches; callbacks receive `unknown`. A correctly annotated callback is rejected. | Instantiate Base UI with `string` and a multiplicity parameter. Compile positive and negative single/multiple cases, and check multiple-value form submission. Keep the default `SelectProps` prop bag compatible with existing multiple-select annotations. | +| RadioGroup erases its string option contract | Numeric selections compile although every rendered option has a string value; callbacks receive `unknown`. | Instantiate the primitive's string props. Compile callback inference and reject numeric selections. | +| Slider, Accordion, and ToggleGroup erase generic information | A scalar Slider callback receives a number/array union; Accordion loses its item type; ToggleGroup widens literal unions to `string`. | Carry the primitive type parameter through each wrapper. Compile number/range, string-array, literal-union, and Slider ref examples. | +| Grouped Autocomplete loses item inference and calls a component directly | Grouped object callbacks infer `unknown`. The wrapper invokes the Base UI root as an ordinary function. | Restore grouped and flat overloads, retain the broad props overload for forwarding, and render the primitive through JSX. Compile grouped/flat consumers and exercise grouped filtering and selection in the docs. | +| Popup titles accidentally intersect with the native `title` attribute | React elements are rejected for Dialog, AlertDialog, and Popover titles despite the explicit `ReactNode` declaration. | Omit native `title` before declaring the content slot. Compile all three and verify dialog accessible names with React element titles. | +| Empty silently discards children | `Create a file` renders no action content. | Render children in the existing contents slot when `contents` is undefined. Preserve explicit contents precedence. Check DOM output and the docs action button. | +| Switch ignores greyUI's disabled fieldset context | A switch inside nested disabled fieldsets fires its callback and lacks disabled state on the control and label. The docs reproduction fails at all four viewport widths. | Resolve inherited disabled state through the same context used by Checkbox. Test nested fieldsets, explicit `disabled={false}`, blocked interaction, and the enabled control. | +| Package checks do not compile consumer contracts | The original declaration build and package audit pass despite the API type defects above. | Compile the same consumer examples against source paths during `typecheck` and real root/subpath package exports during `pack:check`, with `skipLibCheck: false` for published declarations. | +| Browser output contaminates subsequent checks | `npm run check` attempts to format generated Playwright JSON and trace HTML after browser tests. | Ignore `test-results/` and `playwright-report/`, then run the normal checks after browser validation. | + +Type parameter preservation follows the [TypeScript generics guidance](https://www.typescriptlang.org/docs/handbook/2/generics.html). +The Autocomplete rendering change follows React's rule to +[let React call components through JSX](https://react.dev/reference/rules/react-calls-components-and-hooks#never-call-component-functions-directly). +The existing, documented assertion bridging Base UI's overloads remains confined to that +implementation boundary. + +## Validation + +- Baseline `npm run check`: 51 tests pass. New consumer fixtures fail on the original + value/callback/title/grouped-item contracts; the Empty and disabled Switch regressions + fail before implementation changes. +- Final `npm run check`: formatting, lint, TypeScript, and 54 tests pass. +- `npm run build`, `npm run build:docs`, and `npm run pack:check` pass. Package validation + covers strict consumer declaration checking, entrypoint existence, SSR imports, runtime + externals, and bundle budgets. +- Browser suite: 11 tests pass, including existing window and overlay regressions and new + component API flows at 1280, 768, 390, and 320 pixels. +- Reviewed rendered captures of grouped controls, open suggestions, Empty children, and a + dialog with a React title. Compared original and fixed component code using the updated + docs examples. The original code reproduces the missing disabled Switch state at all + four widths; the fixed examples and document remain contained. + +The interactive cloud browser could not open localhost and the standard Playwright browser +CDN timed out. Local browser tests used a temporary npm-distributed Chromium executable via +`PLAYWRIGHT_EXECUTABLE_PATH`; no browser dependency or executable is added to the package. +Validation emitted environment warnings about npm's `http-proxy` setting and conflicting +color-output variables. Cross-browser and assistive-technology testing were not performed. + +## Compatibility notes + +Select and RadioGroup now reject non-string selections that cannot match their string-valued +options. JSX Select usage also rejects mismatched single and multiple value shapes. For a +precisely typed prop bag, use `SelectProps` or `SelectProps`; the unparameterized +type continues to allow both modes. Existing string-valued controls, explicit Empty contents, +flat Autocomplete items, and popup string titles remain supported. diff --git a/docs/src/high-value-components.tsx b/docs/src/high-value-components.tsx index 14d93c2..ae4a113 100644 --- a/docs/src/high-value-components.tsx +++ b/docs/src/high-value-components.tsx @@ -1,6 +1,29 @@ -import { Accordion, Autocomplete, Checkbox, CheckboxGroup, Fieldset, ToggleGroup } from "../../src"; +import { + Accordion, + Autocomplete, + Checkbox, + CheckboxGroup, + Fieldset, + Switch, + ToggleGroup, +} from "../../src"; -const suggestions = ["BeOS R5", "Haiku", "Zeta", "NewOS"]; +const suggestions = [ + { + label: "BeOS family", + items: [ + { id: "beos", name: "BeOS R5" }, + { id: "haiku", name: "Haiku" }, + ], + }, + { + label: "Related systems", + items: [ + { id: "zeta", name: "Zeta" }, + { id: "newos", name: "NewOS" }, + ], + }, +]; function ComponentDemo({ name, children }: { name: string; children: React.ReactNode }) { return ( @@ -17,6 +40,7 @@ export function HighValueComponentDemos() { return (
+ Value arrays preserve their string union in onValueChange. Grid Snap @@ -29,7 +53,7 @@ export function HighValueComponentDemos() {
- + item.name}> @@ -38,20 +62,30 @@ export function HighValueComponentDemos() { No matching suggestion - {(item: string) => ( - - {item} - - + {(group: (typeof suggestions)[number]) => ( + + {group.label} + + {(item: (typeof group.items)[number]) => ( + + {item.name} + + + )} + + )} - Suggestions assist entry; free-form values remain valid. + + Grouped items infer the item type in callbacks; free-form values remain valid. +
+ onValueChange follows the type of value or defaultValue. @@ -95,6 +129,7 @@ export function HighValueComponentDemos() { Managed settings + diff --git a/docs/src/main.tsx b/docs/src/main.tsx index ab8fa74..e4b0a61 100644 --- a/docs/src/main.tsx +++ b/docs/src/main.tsx @@ -393,6 +393,10 @@ function App() { + + Use a string value, or a string array with multiple. Callbacks follow that + choice. + + , + ); + const option = screen.getByRole("option", { name: "Haiku" }); + fireEvent.pointerDown(option, { pointerType: "mouse", button: 0 }); + fireEvent.click(option); + expect(onValueChange).toHaveBeenCalledWith(["beos", "haiku"], expect.anything()); + const form = container.querySelector("form"); + if (!form) throw new Error("Expected form"); + expect(new FormData(form).getAll("themes")).toEqual(["beos", "haiku"]); + }); +}); diff --git a/tests/component-api.types.tsx b/tests/component-api.types.tsx new file mode 100644 index 0000000..74d2169 --- /dev/null +++ b/tests/component-api.types.tsx @@ -0,0 +1,110 @@ +import { Select as SubpathSelect } from "greyui/components/select"; +import { Autocomplete as SubpathAutocomplete } from "greyui/components/autocomplete"; +import type { SelectProps } from "greyui"; +import { createRef } from "react"; +import { expectTypeOf } from "vitest"; +import { + Accordion, + ToggleGroup, + AlertDialog, + Autocomplete, + Dialog, + Popover, + RadioGroup, + Select, + Slider, +} from "greyui"; + +const toggleValues: ("grid" | "snap")[] = ["grid"]; +const multipleProps: SelectProps = { + label: "Themes", + multiple: true, + value: ["beos"], + options: [], +}; +const options = [{ value: "beos", label: "BeOS" }]; + +// Consumer contracts: compiled by typecheck, never mounted by the test runner. +export const componentApiExamples = ( + <> + { + expectTypeOf(value).toEqualTypeOf(); + }} + /> + void value} + /> + {/* @ts-expect-error Select options have string values. */} + + {/* @ts-expect-error Multiple selection requires an array. */} +