Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Suggestion] Async version of .LoadJSON()? #261

Closed
scambier opened this issue May 23, 2024 · 4 comments
Closed

[Suggestion] Async version of .LoadJSON()? #261

scambier opened this issue May 23, 2024 · 4 comments

Comments

@scambier
Copy link
Contributor

Hello :)

I'm using MiniSearch for my Obsidian plugin Omnisearch. It works wonderfully (thank you!), but for a certain category of users with literally thousands of documents, the .LoadJSON() function takes a long time - often more than 10 seconds! - during which the main thread is totally blocked.

I experimented a bit and wrote an async version of this function. The load time is obviously a bit longer, but the user experience is, in my opinion, greatly improved.

Now I understand that supporting large indexes is a non-goal of MiniSearch, buuuuuuut.... 😁 what do you think?

@lucaong
Copy link
Owner

lucaong commented May 28, 2024

Hi @scambier , that's a good idea. I would support an async version of loadJSON for these cases. Your version looks great, I would just look for chances to reduce duplication a bit by factoring out the common part between loadJS and loadJSAsync. I could work on this soon, or alternatively if you want you can send a pull request.

Thank you for the good idea :)

scambier added a commit to scambier/minisearch that referenced this issue May 28, 2024
@scambier
Copy link
Contributor Author

Here it is :)

I did my best to dedupe the code, but the await in a inner loop prevented me to do it as cleanly as I would have liked.

lucaong pushed a commit that referenced this issue May 29, 2024
* Async loading

* import types

* #261 Dedupe and test .loadJSONAsync()

* Added a missing comment
@scambier
Copy link
Contributor Author

Thanks for the merge 🎉

@lucaong
Copy link
Owner

lucaong commented May 30, 2024

Thank you for the contribution! I will soon publish a new release

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants