kentcdodds / match-sorter

Simple, expected, and deterministic best-match sorting of an array in JavaScript
https://npm.im/match-sorter
MIT License
3.73k stars 129 forks source link

Issue with types for nested keys #116

Closed PhilGarb closed 3 years ago

PhilGarb commented 3 years ago

Relevant code or config

const nestedObjList = [
  {
    name: [
      { first: "Janice", last: "Smith" },
      { first: "Jon", last: "Doe" },
    ],
  },
  {
    name: [
      { first: "Fred", last: "Astaire" },
      { first: "Jenny", last: "Doe" },
      { first: "Wilma", last: "Flintstone" },
    ],
  },
];

matchSorter(nestedObjList, 'doe', {
  keys: [
    item => item.name.map(i => i.first), //TypeError
    item => item.name.map(i => i.last), //TypeError
  ],
})

What you did: I followed the documentation. The above example is straight from the docs.

What happened: TypeError

Type '(item: { name: { first: string; last: string; }[]; }) => string[]' is not assignable to type 'KeyOption<{ name: { first: string; last: string; }[]; }>'.
  Type '(item: { name: { first: string; last: string; }[]; }) => string[]' is not assignable to type 'ValueGetterKey<{ name: { first: string; last: string; }[]; }>'.
    Type 'string[]' is not assignable to type 'string'.

Reproduction repository: https://codesandbox.io/s/goofy-hodgkin-nm8u5?file=/src/index.ts

Problem description: I have a data structure with a nested array of objects that I would like to also be match and sortable. The code provided in the documentation worked before 6.0.0 and still works, but now Typescript complains. I guess there is just a small mismatch in the provided types for this use case.

Suggested solution: matchSorter seems to handle the use case just fine. Maybe it is just the accepted type that needs to be adjusted. I think below might be the place where ItemType is specified as string, but it should actually also be an array of strings. I am not sure, whether this would be sufficient though.

https://github.com/kentcdodds/match-sorter/blob/0f731a1646820df2d47b153a575e16d3dc4c3356/src/index.ts#L77-L81

kentcdodds commented 3 years ago

It has a reasonable default. The expectation is that if you want you're safety you provide the type. Is there a way to get type safety without the default and without specifying the type? If we remove the default would that be better?

PhilGarb commented 3 years ago

Thank you for the quick response. Providing a reasonable default is very sensible and I would not remove it. I think the main issue is, that an object with a nested array like above needs to be mapped over to get the key that should be matched. This is what the docs describe. This does however produce an array of strings and not just strings.

I had another look and managed to solve my error by simply creating a union type of string and Array<string>. Please have a look at PR #117. I think this should fix the issue and not interfere with anything else.