Skip to content

[rustdoc] Only generate search DOM elements if the search is actually needed - #160639

Merged
rust-bors[bot] merged 3 commits into
rust-lang:mainfrom
GuillaumeGomez:search-dom
Aug 20, 2026
Merged

[rustdoc] Only generate search DOM elements if the search is actually needed#160639
rust-bors[bot] merged 3 commits into
rust-lang:mainfrom
GuillaumeGomez:search-dom

Conversation

@GuillaumeGomez

Copy link
Copy Markdown
Member

I realized that we were generating the search DOM elements (everything contained into #search) all the times, even when there is no search query parameters in the URL. That seems unnecessary so I reworked the JS a bit to remove that.

r? @lolbinarycat

@rustbot

rustbot commented Aug 6, 2026

Copy link
Copy Markdown
Collaborator

Some changes occurred in HTML/CSS/JS.

cc @lolbinarycat

@rustbot rustbot added A-rustdoc-js Area: Rustdoc's JS front-end S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. T-rustdoc-frontend Relevant to the rustdoc-frontend team, which will review and decide on the web UI/UX output. labels Aug 6, 2026
@@ -16,6 +17,7 @@ define-function: ("collapsed-from-search", [], block {
// Then we collapse the section again...
set-property: ("#implementations-list .implementors-toggle", {"open": "false"})
// Then we run the search.
call-function: ("open-search", {})

@GuillaumeGomez GuillaumeGomez Aug 6, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Ah that was my original "quest": looking at fixing the new flakyness which happened in #159593 (comment). ^^'

View changes since the review

Comment thread tests/rustdoc-gui/sidebar-source-code-display.goml
@rust-log-analyzer

This comment has been minimized.

@GuillaumeGomez

Copy link
Copy Markdown
Member Author

Added the missing Typescript type definition update and CI is now happy.

@GuillaumeGomez

Copy link
Copy Markdown
Member Author

Added comments as suggested for clarification.

@lolbinarycat lolbinarycat left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

looks solid, only a few nits/questions.

View changes since this review

Comment on lines +250 to +251
// If you're browsing the nightly docs, the page might need to be refreshed for
// the search to work because the hash of the JS scripts might have changed.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nit: technically this doesn't just apply to nightly, it's just way more common there

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I copied comment from here. But yeah agreed.

Comment on lines +261 to +269
if (!window.StringdexOnload) {
window.StringdexOnload = [];
}
window.StringdexOnload.push(() => {
loadScript(
getVar("static-root-path") + getVar("search-js"),
sendSearchForm,
);
});

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

the structure of this doesn't make a ton of sense to me in combination, shouldn't we just be initializing it to an array with a single element? otherwise we could hypothetically end up with an array of duplicate initializers?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Just moved existing code, didn't modify anything. I think it's because you can have multiple search indexes loaded in parallel.

Comment thread src/librustdoc/html/static/js/main.js Outdated
Comment on lines +503 to +504
// We load the search input.
window.searchState.inputElement();

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

best i can tell, inputElement() doesn't directly load the search, it sets up a focus handler that loads the search.. why not just load the search and get something more readable and less indirect?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I'll improve the comment as it's quite unclear.

@GuillaumeGomez

Copy link
Copy Markdown
Member Author

Improved the problematic comment. As I mentioned, the rest is just code I moved around.

@lolbinarycat

Copy link
Copy Markdown
Contributor

@bors r+

@rust-bors

rust-bors Bot commented Aug 19, 2026

Copy link
Copy Markdown
Contributor

📌 Commit 730e9a6 has been approved by lolbinarycat

It is now in the queue for this repository.

@rust-bors rust-bors Bot added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Aug 19, 2026
JonathanBrouwer added a commit to JonathanBrouwer/rust that referenced this pull request Aug 19, 2026
…narycat

[rustdoc] Only generate search DOM elements if the search is actually needed

I realized that we were generating the search DOM elements (everything contained into `#search`) all the times, even when there is no search query parameters in the URL. That seems unnecessary so I reworked the JS a bit to remove that.

r? @lolbinarycat
JonathanBrouwer added a commit to JonathanBrouwer/rust that referenced this pull request Aug 20, 2026
…narycat

[rustdoc] Only generate search DOM elements if the search is actually needed

I realized that we were generating the search DOM elements (everything contained into `#search`) all the times, even when there is no search query parameters in the URL. That seems unnecessary so I reworked the JS a bit to remove that.

r? @lolbinarycat
rust-bors Bot pushed a commit that referenced this pull request Aug 20, 2026
…uwer

Rollup of 26 pull requests

Successful merges:

 - #161384 (Bust sccache's cache)
 - #161325 (Bump bootstrap to 1.99.0-beta.1)
 - #161344 (Update the `rustc-perf` submodule)
 - #161366 (Subtree sync for rustc_codegen_cranelift)
 - #159071 ([PAC] Encoder and hash (1/8))
 - #160639 ([rustdoc] Only generate search DOM elements if the search is actually needed)
 - #161068 (Ensure TLS accesses don't call the global allocator through panic (part 3) )
 - #161332 (Some `GlobalCtxt`/`Session` cleanups)
 - #150931 (rustdoc: Always document `#[repr(transparent)]` if `#[rustc_pub_transparent]` is applied)
 - #158999 (doc: list all remove_dir_all fallback targets)
 - #160582 (Add `remove hidden_glob_reexports item breaks downstream` test)
 - #160876 (remove unwrap from write_mir_fn_graphviz)
 - #160927 (Enhance EII UI tests)
 - #161070 (fix arm homogeneous aggregate ABI)
 - #161190 (fix crash in async-drop and async-gen)
 - #161236 (Download auto jobs in citool in parallel)
 - #161254 (Reserve capacity for 3% anon nodes)
 - #161283 (Tighten the language used for documenting `TargetOptions::llvm_abiname`)
 - #161291 (Rename `ProjectionPredicate` and `TraitPredicate`)
 - #161293 (add relnotes 1.98.0)
 - #161299 (Remove a bunch of unnecessary explicit lifetimes)
 - #161307 (make ARM maintainers pingable)
 - #161308 (Add regression test for rustc diagnostic to recognize variables in match guards)
 - #161346 (Don't rely on std traits not being const in tests)
 - #161360 (vec: fixup the name that i forgot)
 - #161367 (Reflect current PR title names for subtree syncs in triagebot no-merges)
@rust-bors
rust-bors Bot merged commit 2b85223 into rust-lang:main Aug 20, 2026
13 checks passed
@rustbot rustbot added this to the 1.100.0 milestone Aug 20, 2026
rust-bors Bot pushed a commit that referenced this pull request Aug 20, 2026
Rollup merge of #160639 - GuillaumeGomez:search-dom, r=lolbinarycat

[rustdoc] Only generate search DOM elements if the search is actually needed

I realized that we were generating the search DOM elements (everything contained into `#search`) all the times, even when there is no search query parameters in the URL. That seems unnecessary so I reworked the JS a bit to remove that.

r? @lolbinarycat
@GuillaumeGomez
GuillaumeGomez deleted the search-dom branch August 20, 2026 19:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-rustdoc-js Area: Rustdoc's JS front-end S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. T-rustdoc-frontend Relevant to the rustdoc-frontend team, which will review and decide on the web UI/UX output.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants