Allow custom filter for suggestions

[sibiraj-s]

Sometimes we might require more than just plain filter, like sort based on best match and etc.

This is a breaking change. I tried to introduce a new prop or something to avoid it, but that would just create confusion by having multiple filters.

[coveralls]

Coverage remained the same at 100.0% when pulling aa19e80 on sibiraj-s:suggestion-filter into e87acf7 on i-like-robots:master.

[i-like-robots]

Thanks for your contribution @sibiraj-s - I think this requirement (to sort suggestions) makes sense and would be a good feature to add to the component. I can see that somebody may want more control over the suggestions, such as scoring and sorting them. Your implementation is neat too 👍

However, I'm reluctant to release a breaking change so soon after the last one as the component is quite widely used so I wonder if there is another approach? For example, do you think it may be adequate to add an optional suggestionsSort prop which could be called on the new array created after suggestionsFilter is called? Maybe like this:

const options = props.suggestions.filter((item) => props.suggestionsFilter(item, state.query)).sort(props.suggestionsSort)

I admit this is not as clean as your implementation but it might be a good enough compromise.

[sibiraj-s]

However, I'm reluctant to release a breaking change so soon after the last one as the component is quite widely used

I agree.

wonder if there is another approach? For example, do you think it may be adequate to add an optional suggestionsSort prop which could be called on the new array created after suggestionsFilter is called? Maybe like this:

We have multiple requirements one is to sort, then based on best match, and we are also looking into group suggestions too. the approach you suggested crossed my mind too. but the default array.sort is limited in features or too much code has to be done (it may become a DRY code). it cannot do all the stuffs that libraries like match-sorter or other libraries could do.

Also having both filters, sorters for a single purpose felt little not neat.

I also saw that v6 took almost a year to get a stable release. I was not even sure whether i should open a PR or not now. IMO, this feature is going to be inevitable, we may need to do this today or in some coming days. as long as we don't do too many breaking changes in the major version, it will be easier for developers to migrate too. we can also do a v7.beta too and wait for some time to release the next stable update.

Meanwhile we can also look into other solutions too. but it should allow more control over the suggestions like grouping (including best matches), sorting, filtering.

[sibiraj-s]

the thought of including the match-sorter inside the lib itself and introduce a sorting strategy or something, so that it won't create any breaking change, but that will increase the lib size, so ignored it.

[sibiraj-s]

this is ugly but, temporarily, something like would not introduce breaking change

if (props.customSuggestionsFilter) {
  options = props.customSuggestionsFilter(state.query, props.suggestions);
} else {
  options = props.suggestions.filter((item) =>
    props.suggestionsFilter(item, state.query)
  );
}

naming can be discussed.

[i-like-robots]

I think adding a condition like this is fine, so long as it can be explained clearly in the documentation. For this reason calling it a "filter" may not be quite right as the proposed callback is potentially more powerful than this 🤔

Perhaps suggestionsTransform might more clearly indicate the difference between the two. I'll take a look around and try and come up with some suitable verbs.

if (props.suggestionsTransform) {
  options = props.suggestionsTransform(state.query, props.suggestions);
} else {
  options = props.suggestions.filter((item) =>
    props.suggestionsFilter(item, state.query)
}

[sibiraj-s]

suggestionsTransform that sounds like a good option too, also how about deprecating the existing filter? we can remove it in the next major version and replace it with new function as default?

[sibiraj-s]

@i-like-robots can we settle on something and get this merged? we need this in coming weeks

[i-like-robots]

Let's go with suggestionsTransform as discussed above. I think leaving the existing filter is OK for now, it can be formally deprecated as part of planning for the next major version. If you're happy to make the changes I'd be very grateful.

[sibiraj-s]

@i-like-robots made the changes and added tests. let me know if any test cases are missing. I will add.

[i-like-robots]

This looks great, thank you @sibiraj-s