Add option to return all items when querying for an empty string

[Gelio]

I noticed in autosuggest-trie.js, that when the query string is empty, an empty array is always returned:

function getPhraseIndices(phrase, limit) {
  phrase = phrase.trim();

  if (phrase === '') {
    return [];
  }
  ...
}

It probably is the default behavior for such a list. However, it would be a lot more flexible if there was an option you could set when creating the structure to make it the opposite - return all the items for an empty phrase.

My use case for that is an autocomplete functionality, where I would like to return all the items at the beginning and filter it down the road.

I don't think this would be a detrimental change to the project. As to how to implement it without introducing breaking changes, I suggest changing the parameters on create function, perhaps for the following:

function create(items, textField, options) {

And then unwrap the options, having backwards compatibility in mind:

let itemsComparator;
let returnAllWhenEmptyPhrase = false;

if (typeof options === 'function') {
  itemsComparator = options;
} else {
  { itemsComparator, returnAllWhenEmptyPhrase } = options;
}

I can submit a Pull Request if you find this functionality welcome. I would really want that, as it's the only feature missing for my project. If this doesn't get incorporated into the project, I will, unfortunately, have to refer to the use of another library.

By the way, good job on this library. I extremely appreciate your effort!

[moroshko]

@Gelio Thanks a lot for a properly submitted issue!

Your suggestion makes sense, but I'm curious to know what stops you from doing something like this?

if (query.trim() === '') {
  items = allItems;
} else {
  items = trie.getMatches(query);
}

[Gelio]

@moroshko thank you for your time! Actually, that's the workaround I have been using for now. I just thought it would be a good enhancement to this library, as people who use it for similar purposes as me would have it already implemented as a handy declarative option, instead of that workaround.

If you decide it isn't needed I will accept it and hold no grudge. I just mean to help

[moroshko]

@Gelio How about this:

createTrie(items, textKey, { blankQueryMatches: allItems })

?

Note that the API changed a bit in v2.0. I'd appreciate your feedback on the new API.

[Gelio]

That looks good! I will look into the 2.0 API as soon as I get home, good job

[Gelio]

@moroshko I've seen the new API, looks solid, extensibility is nicely included now. I believe what you proposed with that blankQueryMatches option also is valid. Are you going to implement this yourself, or may I add this (along with necessary tests) and make a pull request?

This would be my first open source contribution, so if you would give me the opportunity I would be grateful :) no worries, I understand if you want to do it yourself

[moroshko]

Go ahead! I'll be happy to accept a PR.

[Gelio]

I'm wondering if we should slice the array if a limit is specified when using a blank query. The same goes for sorting the array - do we sort it when using a blank query?

To my mind, it should return all the items specified when creating a trie, however, they should be sorted. Therefore it can sort the items at the beginning if the comparator function is provided.

[moroshko]

Hmm.. Now I feel that blankQueryMatches only introduces an unnecessary complexity to the surface area of this library, without adding much value in return.

It doesn't make sense to me to limit the blank query matches. Same with sorting. I think this logic should live outside of this library.

When I think more about this, I feel like this:

if (query.trim() === '') {
  items = allItems; // you can slice or limit here, it's totally up to you!
} else {
  items = trie.getMatches(query);
}

is actually the right way to use autosuggest-trie (not a workaround).

The purpose of this library is to get matches based on a query. If you don't have a query (i.e. the query is blank) you shouldn't be trying to get matches. That's my mental model. I hope it makes sense.

I'll have to pass on this one. Sorry if I wasted your time. Somehow, I hope it was a positive experience anyway.

I just want to thank you again for taking the time to raise the issue, propose and implement a solution, and having a positive attitude. Please don't let this experience to stop you from contributing to Open Source in the future.

[Gelio]

I just wanted to say that I'm actually content with how it went. It's my first open source contribution, at least a mental one, No problem, I also noticed that when implementing this, that it probably does too much, as you said. It's a valid point. I will continue to use my workaround in the app.

Thank you for being a considerate project maintainer! It was a pleasure