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

feat: Add "latest" and "related" search. #2055

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions assets/js/globals.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,3 +15,7 @@ export function getSidebarNodes () {
export function getVersionNodes () {
return window.versionNodes || []
}

export function getSearchNodes () {
return window.searchNodes || []
}
72 changes: 64 additions & 8 deletions assets/js/search-page.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import lunr from 'lunr'
import { qs, escapeHtmlEntities, isBlank, getQueryParamByName, getProjectNameAndVersion } from './helpers'
import { setSearchInputValue } from './search-bar'
import searchResultsTemplate from './handlebars/templates/search-results.handlebars'
import { getSearchNodes } from './globals'

const EXCERPT_RADIUS = 80
const SEARCH_CONTAINER_SELECTOR = '#search'
Expand All @@ -27,30 +28,85 @@ function initialize () {
const pathname = window.location.pathname
if (pathname.endsWith('/search.html') || pathname.endsWith('/search')) {
const query = getQueryParamByName('q')
search(query)
const queryType = getQueryParamByName('type')
search(query, queryType)
}
}

async function search (value) {
async function search (value, queryType) {
if (isBlank(value)) {
renderResults({ value })
} else {
setSearchInputValue(value)

const index = await getIndex()

try {
// We cannot match on atoms :foo because that would be considered
// a filter. So we escape all colons not preceded by a word.
const fixedValue = value.replaceAll(/(\B|\\):/g, '\\:')
const results = searchResultsToDecoratedSearchItems(index.search(fixedValue))
let results = []
const searchNodes = getSearchNodes()

if (['related', 'latest'].includes(queryType) && searchNodes.length > 0) {
results = await remoteSearch(value, queryType, searchNodes)
Copy link

@ruslandoga ruslandoga Jan 22, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a couple nitpicks :)

Can we have a race condition here? When the previous request returns after the current request and updates the items to stale results. I think it's possible with multiple HTTP/1.1 connections, but not sure about multiple streams on the same HTTP/2 connection, are they guaranteed to be ordered? Or maybe JS runtime resolves it in some way?

Also, do we need to debounce on remote search or check for response.ok and results.length > 0?

For some reason I decided to do these things in ruslandoga#1 but I don't remember if I actually had these problems or was just playing it safe ...

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. I'm sure you're right. As you can probably tell it's been almost a decade since I wrote any JavaScript so I'm still getting the hang of the new idioms.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So looking at the code more carefully, it appears that the search function is only called on page load, so should only be run once in the page's lifecycle.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Additionally, the search result handlebars template takes care of whether any results were actually returned.

} else {
results = await localSearch(value)
}

renderResults({ value, results })
} catch (error) {
renderResults({ value, errorMessage: error.message })
}
}
}

async function localSearch (value) {
const index = await getIndex()

// We cannot match on atoms :foo because that would be considered
// a filter. So we escape all colons not preceded by a word.
const fixedValue = value.replaceAll(/(\B|\\):/g, '\\:')
return searchResultsToDecoratedSearchItems(index.search(fixedValue))
}

async function remoteSearch (value, queryType, searchNodes) {
let filterNodes = searchNodes

if (queryType === 'latest') {
filterNodes = searchNodes.slice(0, 1)
}

const filters = filterNodes.map(node => `package:=${node.name}-${node.version}`).join(' || ')
jimsynz marked this conversation as resolved.
Show resolved Hide resolved

const params = new URLSearchParams()
params.set('q', value)
params.set('query_by', 'title,doc')
params.set('filter_by', filters)

const response = await fetch(`https://search.hexdocs.pm/?${params.toString()}`)
const payload = await response.json()

if (Array.isArray(payload.hits)) {
return payload.hits.map(result => {
const [packageName, packageVersion] = result.document.package.split('-')

const doc = result.document.doc
const excerpts = [doc]
const metadata = {}
const ref = `https://hexdocs.pm/${packageName}/${packageVersion}/${result.document.ref}`
const title = result.document.title
const type = result.document.type

return {
doc,
excerpts,
metadata,
ref,
title,
type
}
})
} else {
return []
}
}

function renderResults ({ value, results, errorMessage }) {
const searchContainer = qs(SEARCH_CONTAINER_SELECTOR)
const resultsHtml = searchResultsTemplate({ value, results, errorMessage })
Expand Down
Loading