itemsapi / itemsjs

Extremely fast faceted search engine in JavaScript - lightweight, flexible, and simple to use
Apache License 2.0
346 stars 41 forks source link

Update to Lunr 2 #29

Closed domoritz closed 3 years ago

domoritz commented 5 years ago

It would be great if itemsjs could be updated to the latest version of lunr. I'm running into some issues with the old set implementation and it looks like the new version does not have this issue anymore.

cigolpl commented 5 years ago

@domoritz thanks for feedback! I was considering Lunr 2.x but there was slight difference between Lunr 1.x API. They were not compatible as far as I remember and Lunr 1.x was working good enough for ItemsJS API at that time.

What kind of issues were you facing with the older Lunr here ?

domoritz commented 5 years ago

@cigolpl Thank you for the quick response. The issue I am encountering is that when I add full-text search to my app, I run into an infinite loop (see below). What's odd is that the field it's searching over is not in my searchableFields list.

screen shot 2018-11-24 at 12 00 52
cigolpl commented 5 years ago

@domoritz thanks for an example! If you could reproduce that problem with your items, configuration, search configuration, expected result (the smaller input provided the easier debuging) then it'll be much easier to find a root cause.

renebakx commented 4 years ago

I got the indexer part working for Lunr 2.3, and I'm working on extending the searcher with pipeline support. But that's giving me a little more headache. Mostly cause my Javascript skills are too rusty to fix with WD40 ;)

Follow my progress https://github.com/renebakx/itemsjs

Will make a proper pull request if I feel it's mature enough.

renebakx commented 4 years ago

@cigolpl Documented my progress here : https://github.com/renebakx/itemsjs/blob/master/docs/changes.md

So far everything works, but I wanted move towards implementing Lunr 2.x using it's Plugin model But I think that would require a breaking change in the way the Fulltext class/object is used.

How are you looking at this?

cigolpl commented 4 years ago

Hey @renebakx, I've seen your document. Personally:

1) I'd upgrade ItemsJS to Lunr 2.x and check if that passes the current tests 2) In terms of searchableFields I'd keep API the same. Keeping fields values as an array of strings. We can additionally accept them as an array of objects i.e.:

{
  "name": {
    "boost": 10
  },
  "category": {
    "boost": 8
  }
}

So we are not breaking API and the user has a choice to use boost or maybe another options in the future

3) searchMatcherConfiguration sounds like a good option 4) I'd keep yet isExactSearch as some devs use that feature now. If we can solve it with searchMatcherConfiguration then I'd mark it as deprecated and remove it in next release.

renebakx commented 4 years ago

Thanks for your answers!

  1. Running tests in javascript is new to me as a more PHP orientated developer, but I will figure it out ;) The search is working because I was brave (or stupid) enough to move it into production ;)

  2. Check.. missed that part and thought it was not in the current itemsJS, will rebuild it to this instead. Makes more sense and is future proof if other config options for fields come available!

  3. Cool :)

  4. I can keep isExactSearch, but I have to figure out how to configure that with the 2.x version, if understand it correctly it's all moved to the pipeline functions but that would mean I have to refactor the way the Fulltext search is implemented in itemsJS. Have to look into that, but in a way you can sort of configure it with the matcher, but the matcher is on search operation, while the exactSearch is on indexer level.

cigolpl commented 3 years ago

@domoritz I'll probably upgrade to Lunr 2 to make native search better but before that I consider making full text search optionally pluggable. There is so many pure full text search on the market:

If you can plug your own full text to itemsjs then you have full control over all features (stemming, tokenizing, stop-words, etc) For developers who does not have more complex requirements the basic native search will be enough.

What do you think ?

domoritz commented 3 years ago

I'm a huge fan of composition so that sounds great.

cigolpl commented 3 years ago

@domoritz the integration with Lunr 2.x is now possible through simple integration. The example is here https://github.com/itemsapi/itemsjs/blob/master/docs/lunr2-integration.md. The version 2.1.1 is required.

How it works:

Bartel-C8 commented 3 years ago

@cigolpl : Would it be possible to make the id property configurable in itemsjs as well (as it actually is in lunr2, by specifying the ref)?

cigolpl commented 3 years ago

@Bartel-C8 it's not implemented but it'd be possible. If you need such a feature please create a new issue