Skip to content

Load time optimization: download search index asynchronously; make cacheable #2700

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.

Sign up for GitHub

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

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open

Load time optimization: download search index asynchronously; make cacheable #2700

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
e8bf846
Include content in search_index.js path; make script async
xal-0 May 6, 2025
5e7c196
Listen for the load event instead of polling for search index
xal-0 May 6, 2025
7da80d6
Fix Julia formatting issues
xal-0 May 6, 2025
f80982f
Run code formatters on JS and Julia
xal-0 May 7, 2025
6c98c4f
Use right version of prettier this time
xal-0 May 7, 2025
cae8623
Add changelog entry
xal-0 May 7, 2025
69f530a
Update CHANGELOG.md
mortenpi May 7, 2025
3cea893
Merge branch 'master' into async-search-index
fingolfin May 7, 2025
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
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

* Refactored `deploydocs` internals to allow dispatch on the `versions` keyword, intended as a non-public API for DocumenterVitepress which cannot use the default versioning mechanism during deployment ([#2695]).
* Use different banners for dev and unreleased docs ([#2382], [#2682])
* The search index now loads asynchronously, and can be cached indefinitely. ([#2702], [#2700])
Copy link
Collaborator

Choose a reason for hiding this comment

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

@xal-0 those PRs also need to be added to the list at the end of the file (make sure to pull our changes first :-) )

Copy link
Member

Choose a reason for hiding this comment

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

Note: one can run make changelog, which will update the references in CHANGELOG.md automatically.


### Fixed

Expand Down
4 changes: 3 additions & 1 deletion assets/html/js/search.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -610,7 +610,9 @@ function waitUntilSearchIndexAvailable() {
runSearchMainCode();
} else {
console.warn("Search Index not available, waiting");
setTimeout(waitUntilSearchIndexAvailable, 1000);
document
.getElementById("documenter-search-index-script")
.addEventListener("load", waitUntilSearchIndexAvailable);
}
}

Expand Down
48 changes: 35 additions & 13 deletions src/html/HTMLWriter.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -771,7 +771,6 @@ function render(doc::Documenter.Document, settings::HTML = HTML())
end

ctx = HTMLContext(doc, settings)
ctx.search_index_js = "search_index.js"
ctx.themeswap_js = copy_asset("themeswap.js", doc)
ctx.warner_js = copy_asset("warner.js", doc)

Expand Down Expand Up @@ -800,9 +799,34 @@ function render(doc::Documenter.Document, settings::HTML = HTML())
copy_asset("themes/$(theme).css", doc)
end

size_limit_successes = map(collect(keys(doc.blueprint.pages))) do page
function page_navnode(page)
Copy link
Member

Choose a reason for hiding this comment

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

Bikeshed, but let's move this out of the function body perhaps. I am not a huge fan of declaring functions within functions. I think it just needs to take doc::Document as the first argument?

idx = findfirst(nn -> nn.page == page, doc.internal.navlist)
nn = (idx === nothing) ? Documenter.NavNode(page, nothing, nothing) : doc.internal.navlist[idx]
return nn, idx
end

foreach(keys(doc.blueprint.pages)) do page
nn, idx = page_navnode(page)
@debug "Indexing $(page) [$(repr(idx))]"
generate_index!(ctx, nn)
end
Comment on lines +808 to +812
Copy link
Member

Choose a reason for hiding this comment

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

Mmm. I don't think this works. A lot of the pushing into .search_index happens during the DOM generation deep in the call tree, i.e. downstream from render_page. So right now a bunch of stuff is missing (e.g. docstrings).

But yea, we have a chicken-and-egg situation here, where in order to get the full search index, we need to render everything, but we don't know how to render it because we don't know the search index hash yet, since we haven't generated the index yet.

I do think that with some reorganization of the code, we could somehow generate all the DOM first, and then inject the search index file name later. Although one nice property of the current structure is that we don't need to keep the DOM of all the pages in memory at once, which I am not sure how we could keep.

What I might suggest (unless you have a good simple idea of how to tackle this) is to undo the hashing related changes for now. There's also another issue with it -- MultiDocumenter, for example, relies on the search_index.js file being where it is, so we may need to think about this change a bit more anyway. But we should definitely get the front end loading changes in.


# Make an attempt at avoiding spurious changes to the search index hash.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Would this be accurate?

Suggested change
# Make an attempt at avoiding spurious changes to the search index hash.
# Sort the index to avoid spurious changes to the search index hash caused
# by difference in the order of entries

sort!(ctx.search_index; by = r -> (r.src, r.fragment, r.text))
search_index_buf = let io = IOBuffer()
println(io, "var documenterSearchIndex = {\"docs\":")
# convert Vector{SearchRecord} to a JSON string + do additional JS escaping
println(io, JSDependencies.json_jsescape(ctx.search_index), "\n}")
take!(io)
end
# Put the index's hash in its filename so it can have a long max-age.
ctx.search_index_js = "search_index_$(dataslug(search_index_buf)).js"
open(joinpath(doc.user.build, ctx.search_index_js), "w") do io
write(io, search_index_buf)
end

size_limit_successes = map(collect(keys(doc.blueprint.pages))) do page
nn, idx = page_navnode(page)
@debug "Rendering $(page) [$(repr(idx))]"
render_page(ctx, nn)
end
Expand Down Expand Up @@ -837,12 +861,6 @@ function render(doc::Documenter.Document, settings::HTML = HTML())
# Check that all HTML files are smaller or equal to size_threshold option
all(size_limit_successes) || throw(HTMLSizeThresholdError())

open(joinpath(doc.user.build, ctx.search_index_js), "w") do io
println(io, "var documenterSearchIndex = {\"docs\":")
# convert Vector{SearchRecord} to a JSON string + do additional JS escaping
println(io, JSDependencies.json_jsescape(ctx.search_index), "\n}")
end

write_inventory(doc, ctx)

return generate_siteinfo_json(doc.user.build)
Expand Down Expand Up @@ -1027,7 +1045,7 @@ function render_head(ctx, navnode)
:src => RD.requirejs_cdn,
Symbol("data-main") => relhref(src, ctx.documenter_js),
],
script[:src => relhref(src, ctx.search_index_js)],
script[:src => relhref(src, ctx.search_index_js), :async, :id => "documenter-search-index-script"],

script[:src => relhref(src, "siteinfo.js")],
script[:src => relhref(src, "../versions.js")],
Expand Down Expand Up @@ -1700,10 +1718,6 @@ end
function domify(dctx::DCtx)
ctx, navnode = dctx.ctx, dctx.navnode
return map(getpage(ctx, navnode).mdast.children) do node
rec = searchrecord(ctx, navnode, node)
if !isnothing(rec)
push!(ctx.search_index, rec)
end
domify(dctx, node, node.element)
end
end
Expand Down Expand Up @@ -1848,6 +1862,14 @@ function domify(::DCtx, ::Node, rawnode::Documenter.RawNode)
return rawnode.name === :html ? DOM.Tag(Symbol("#RAW#"))(rawnode.text) : DOM.Node[]
end

function generate_index!(ctx::HTMLContext, navnode::Documenter.NavNode)
return foreach(getpage(ctx, navnode).mdast.children) do node
rec = searchrecord(ctx, navnode, node)
if !isnothing(rec)
push!(ctx.search_index, rec)
end
end
Comment on lines +1866 to +1871
Copy link
Member

Choose a reason for hiding this comment

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

foreach doesn't return anything, so it looks a bit weird to me.

Suggested change
return foreach(getpage(ctx, navnode).mdast.children) do node
rec = searchrecord(ctx, navnode, node)
if !isnothing(rec)
push!(ctx.search_index, rec)
end
end
foreach(getpage(ctx, navnode).mdast.children) do node
rec = searchrecord(ctx, navnode, node)
if !isnothing(rec)
push!(ctx.search_index, rec)
end
end
return nothing

Also, as a side note: I don't mind having foreach-es, but I don't really find them particularly idiomatic. I'd just write this as for node in getpage(ctx, navnode).mdast.children ... end.

end

# Utilities
# ------------------------------------------------------------------------------
Expand Down
Loading