optimize dropdown search#3768
Conversation
AnnMarieW
requested review from
KoolADE85,
T4rk1n,
camdecoster and
ndrezn
as code owners
camdecoster
left a comment
camdecoster
left a comment
There was a problem hiding this comment.
This seems fine, but you'll need to update references from search_order to searchOrder.
| search.tokenizer = TOKENIZER; | ||
|
|
||
| indexes.forEach(index => { | ||
| search.addIndex(index); |
There was a problem hiding this comment.
Does it matter if the index include here is the same as the uidFieldName passed into the constructor? I ask because indexes always contains 'value' and that's the same as the uidFieldName used in the constructor.
There was a problem hiding this comment.
'value' in the constructor is only used as the document uid field in js-search. It is not automatically indexed for searching, so we still need search.addIndex('value') here.
This is unchanged from previous version (old line 75-77)
| options: SanitizedOptions, | ||
| searchValue?: string, | ||
| search_order?: 'index' | 'original' | ||
| searchOrder?: string |
There was a problem hiding this comment.
Why remove the union type here? In fact, you could reference the existing type in components/dash-core-components/src/types.ts.
There was a problem hiding this comment.
Good call! It's better to use the correct prop types.
| options: SanitizedOptions, | ||
| searchValue?: string, | ||
| search_order?: 'index' | 'original' | ||
| searchOrder?: string |
There was a problem hiding this comment.
I think you'll need to update references to this property throughout the repo.
There was a problem hiding this comment.
I'm not sure what you mean here. But if the type is defined like this, does it have to change anywhere else?
searchOrder?: DropdownProps['search_order']
| } | ||
| // Preserve original option order | ||
| if (searchOrder === 'original') { | ||
| const resultSet = new Set(results.map(option => option.value)); |
There was a problem hiding this comment.
Why do you use map here? Does it allow for faster lookups when only using value?
There was a problem hiding this comment.
The map just makes it more robust. It converts the results into a Set of option values instead of object references.
2 participants
Closes #3616
This PR changes the
dropdownSearch.tsso it builds the search index once during option sanitization and reuses it instead of rebuilding the index on every search input update.I tested the example included in issue 3616 with 80,000 options and search performance is now comparable to Dash 3.4.