feat: use flexsearch for all local searches #997
No reviewers
Labels
No Label
area/internal-code
changes requested
confirmed
dependencies
duplicate
good first issue
help wanted
hosting
invalid
kind/bug
kind/feature
question
wontfix
No Milestone
No project
No Assignees
3 Participants
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: vikunja/frontend#997
Loading…
Reference in New Issue
No description provided.
Delete Branch "feature/flexsearch"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
This PR introduces flexsearch when searching for labels, lists and namespaces locally.
To discuss:
There's quite a bit of duplication I created here, not sure how to handle this in a better way.
Also I noticed that, while pretty similar, all three (labels, namespaces, lists) handle storing items in vuex slightly different. Labels are stored in a
labels
object in the state with the id as key, lists are stored directly in the state without any object in the middle of it and namespaces are stored in anamespaces
array.That has grown historically, but I think it should be refactored at some point.
@dpschen Do you maybe have an idea how to handle this in a good way?
In general:
What I don't like:
@ -29,0 +31,4 @@
})
return resultIds
.filter((value, index, self) => self.indexOf(value) === index && !labelIdsToHide.includes(value))
Use
includes()
How? I'm using this to filter out duplicate Ids.
Ignore. Sry, I misread
@ -0,0 +1,19 @@
import {createNewIndex, withId} from '@/indexes/index'
In these three files the duplication can be avoided by creating a factory function.
Something like:
and then:
On another thought:
The
createNewIndexer
should probably just be defined inside the index file alreadyThat sounds like a really good idea. I've moved it all to a factory function.
@ -3,6 +3,7 @@ import {setLoading} from '@/store/helper'
import {success} from '@/message'
import {i18n} from '@/i18n'
import {getLabelsByIds, filterLabelsByQuery} from '@/helpers/labels'
import {add, remove, update} from '../../indexes/labels'
use
@
Done.
@ -35,2 +39,4 @@
return typeof list === 'undefined' ? null : list
},
searchList: state => (query, includeArchived = false) => {
if (query === '' || query === null) {
Don' repeat all the search validation. Instead put that in the search function.
(you repeated this validatin multiple times)
In the indexer module? The problem here is it does not just return an array, but another object with the results in them (coming from flexsearch). Should I recreate that?
I meant putting the query check in the search function that you create =)
Just to clarify, here? https://kolaente.dev/vikunja/frontend/src/branch/feature/flexsearch/src/indexes/index.ts#L39
Yes!
Okay, but what should I return if the query is empty? I can't really return an empty array because the caller expects the object with the results from flexsearch.
Or should I return
null
and check that in the caller?Return
null
(I would have usedundefined
, but after thinking about itnull
seems to make more sense…?).After the call of
search()
you usually iterate over the results. You can use optional chaining to check is you can do that (result?.map( [...] ) || FALLBACK
)Is that what you mean?
yeah pretty much.
Moved the check and simplified accessing it.
@ -37,0 +51,4 @@
})
return resultIds
.filter((value, index, self) => self.indexOf(value) === index && value > 0)
use
includes()
@ -100,0 +112,4 @@
const namespacesFromIndex = search(query)
const resultIds = []
namespacesFromIndex.forEach(r => {
Use
.map()
Done.
@ -100,0 +117,4 @@
})
return resultIds
.filter((value, index, self) => self.indexOf(value) === index && value > 0)
Use
.includes()
@ -100,0 +119,4 @@
return resultIds
.filter((value, index, self) => self.indexOf(value) === index && value > 0)
.map(id => {
const namespaceIndex = state.namespaces.findIndex(n => n.id === id)
Use
.find()
Done.
Hi konrad!
Thank you for creating a PR!
I've deployed the changes of this PR on a preview environment under this URL: https://997-featureflexsearch--vikunja-frontend-preview.netlify.app
You can use this url to view the changes live and test them out.
You will need to manually connect this to an api running somehwere. The easiest to use is https://try.vikunja.io/.
Have a nice day!
@ -27,2 +26,2 @@
return !labelIds.includes(id) && title.toLowerCase().includes(labelQuery)
})
return search(query)
?.map(r => r.result)
Use flatMap.
Oh, I didn't know that exists. Done.
@ -5,1 +5,4 @@
import {getLabelsByIds, filterLabelsByQuery} from '@/helpers/labels'
import {createNewIndexer} from '@/indexes'
const {add, remove, update} = createNewIndexer('labels', ['title', 'description'])
I like that =)
@ -47,3 +55,4 @@
removeNamespaceById(state, namespaceId) {
for (const n in state.namespaces) {
if (state.namespaces[n].id === namespaceId) {
remove(state.namespaces[n])
Picky:
remove
after splice (if why ever splice failsremove
will correctly not be reachedDone.
@ -100,0 +112,4 @@
.flat()
.filter((value, index, self) => self.indexOf(value) === index && value > 0)
.map(id => {
const namespace = state.namespaces.find(n => n.id === id)
Acutally: use getNamespaceById getter
Done.