Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

matchSorter does not accept readonly arrays. #128

Closed
diesieben07 opened this issue Sep 15, 2021 · 3 comments · Fixed by #129
Closed

matchSorter does not accept readonly arrays. #128

diesieben07 opened this issue Sep 15, 2021 · 3 comments · Fixed by #129
Labels

Comments

@diesieben07
Copy link
Contributor

  • match-sorter version: 6.3.0
  • node version: 14.17.0
const input: readonly string[] = ['foo', 'bar'];
const matchSorted = matchSorter(input, 'f'); // does not compile, plain array is required

Problem description:
The type signature of matchSorter is overly restrictive. Even though it does not modify its input array, it does not accept readonly arrays.
To work around this one needs to either do a cast (which basically disables type safety) or copy the array unnecessarily before passing it to matchSorter.

Suggested solution:
matchSorter should accept a ReadonlyArray instead of a plain Array.

I am creating this issue before a pull request, to first have a discussion about the scope. This readonly issue not only affects the input array (although that is where I discovered it), it also applies to for example MatchSorterOptions.keys. Changing it there however would be a breaking change (as opposed to the type signature of matchSorter).

@kentcdodds
Copy link
Owner

I think we should support a regular array as well as a readonly array. PR is welcome.

@diesieben07
Copy link
Contributor Author

A regular array can also be passed if a readonly array is expected. The following compiles fine:

function acceptsArray(array: ReadonlyArray<string>) { }

const plainArray: string[] = ['foo'];
acceptsArray(plainArray); // compiles!

I'll make a PR later today!

@github-actions
Copy link

🎉 This issue has been resolved in version 6.3.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants