Skip to content

fix(types): accept readonly items in selection hooks - #1702

Open
minwookshin wants to merge 1 commit into
downshift-js:masterfrom
minwookshin:fix/readonly-hook-items
Open

minwookshin wants to merge 1 commit into
downshift-js:masterfrom
minwookshin:fix/readonly-hook-items

Conversation

@minwookshin

Copy link
Copy Markdown

Pull Request

What

Accept readonly arrays and as const tuples in useSelect, useCombobox and useMultipleSelection.

Closes #1030.

Why

These hooks read the supplied arrays without mutating them, but the mutable-only declarations reject readonly inputs and lose tuple item inference.

How

Widen the array parameters in the hook declarations and their read-only helpers. useMultipleSelection also exposes readonly state and callback arrays, since these can be the original caller-supplied arrays. There are no runtime changes.

Type compatibility: callers that require mutable useMultipleSelection.selectedItems or callback arrays need to accept readonly arrays or make a copy. Mutable inputs remain supported. The multiple-select example's filtering helper is updated accordingly.

Changes

  • Add six frozen-array regressions covered by both Jest and test:ts; the readonly calls fail type checking before the change.
  • Verify tuple inference and mutation rejection through the built public declarations.
  • Local validation: 1,116 unit tests with 100% coverage, 8 package tests, SSR, source/example type checks, declaration generation with the project's TypeScript 5.9, changed-file ESLint and Prettier pass.
  • Browser checks: 24/25 pass. The compiled-combobox selection case expects Aqua but receives Black; it also fails on unmodified 1bb8b75 (which additionally failed the uncompiled variant).

Existing tooling limitations were checked against that same base: the default ESLint parser exhausts its heap, and kcd-scripts invokes TypeScript 4.9 for declarations despite the project's 5.9 dependency. With the project's current parser, full-repository lint has the same 73 errors and 19 warnings on both versions, with no added diagnostics. The pre-commit hook also rejects the existing bundler module resolution; it was skipped after the changed-file checks passed with the project parser. These unrelated tooling issues are unchanged.

Checklist

  • Documentation
  • Tests
  • TypeScript Types
  • Ready to be merged

Leaving the final item unchecked pending review of the type compatibility change and the baseline validation failures above.

Allow readonly arrays and tuples in select, combobox and multiple-selection
inputs. Preserve readonly array ownership in multiple-selection state and
callbacks, and cover frozen inputs and inferred item types.

Closes downshift-js#1030

BREAKING CHANGE: useMultipleSelection selectedItems state and callback arrays
are readonly. Callers requiring mutable arrays must copy them or accept
readonly arrays.
@minwookshin

Copy link
Copy Markdown
Author

Could you approve the pending validate workflow? Local results and the failures reproduced on the base branch are noted above. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

TypeScript: have items and selectedItems as Readonly

1 participant