Feature: Update search to use PageFind for indexing and retrieval. #1084
Conversation
fchasen
force-pushed
the
pagefind_search
branch
from
b63cdcb to
8126bee
Compare
mphstudios
left a comment
mphstudios
left a comment
There was a problem hiding this comment.
@fchasen this pull-request will need recent changes on main and to update the package version rc candidate number before being merged.
- Adds the `pagefind` library and `removes` lunr - Updates the search plugin to index HTML pages and figures using a new `SearchIndex` class - Output the search index to `_search` along with the frontend API files that PageFind builds - Adds pagefind data attributes as meta tags and on `.quire-page` to pass page metadata to the index - Updates the existing search UI to use the pagefind API - Adds the Sinon library for creating test mocks - Adds tests for SearchIndex using mocks for the pagefind methods - Bundles the pagefind search API and imports it when `loadSearchData` is called, so pagefind will load the index using wasm.
fchasen
force-pushed
the
pagefind_search
branch
from
9839f9d to
ab095cd
Compare
|
@fchasen I'm getting duplicate search results when I have the url: 'https://quire.reclaim.hosting/test-pagefind/'When I run 
But when I run 
You can see the deployed site here: https://quire.reclaim.hosting/test-pagefind/. Let me know if there's anything else I can provide to help track down the issue. |
|
I pushed one commit to fix a missing paren, but otherwise can confirm that the duplication bug is fixed. From my QA standpoint this is ready to merge, but @mphstudios or @cbutcosk will need to look at it to resolve the I don't know what's supposed to be what with these versions, but here's what I'm seeing in the PR:
|
| resultsContainer.appendChild(clone) | ||
| const searchInstance = window.QUIRE_SEARCH | ||
| window._searchResults = searchInstance.search(query) | ||
| .then((searchResults) => { |
There was a problem hiding this comment.
@fchasen Thanks for these updates! Is it possible to change these chained promises to a series of await statements? The latter will read more idiomatically within the async function.
Otherwise looked good to me. The CI failure is an image deprecation issue that we should be able to resolve with a PR to main ahead of this merge.
# Conflicts: # package-lock.json
|
Nota bene: this pull request depends on #1126 being merged to fix failing end-to-end tests. |
Checklist
Please put an
Xwithin the brackets that apply[X].I have read the CONTRIBUTING.md file
I have made my changes in a new branch and not directly in the main branch
This pull request is ready for final review by the Quire team
Please briefly describe the goal of this pull request and how it may impact Quire's functionality.
This switches from using Lunr for search to using PageFind. PageFind allows for more complex indexes that include figures and object data, search ranking of page sub-results and faster retrieval using their API and the wasm index that it outputs.
Does this pull request necessitate changes to Quire's documentation?
These changes only update the indexing and search API, maintaining the existing search UI and configuration.
Please describe the changes you made, and call out any details you think are particularly relevant for the Quire team to note in their review.
pagefindlibrary andremoveslunrSearchIndexclass_searchalong with the frontend API files that PageFind builds.quire-pageto pass page metadata to the indexloadSearchDatais called, so pagefind will load the index using wasm.