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

Improve loading of crates.js and sidebar-items.js #98124

Merged
merged 1 commit into from
Jun 21, 2022

Conversation

jsha
Copy link
Contributor

@jsha jsha commented Jun 15, 2022

Now that the "All Crates" dropdown is only rendered on the search results page,
there is no need to load crates.js on most pages. Load it only on crate pages.
Also, add the defer attribute so it does not block HTML parsing.

For sidebar-items.js, move the script tag to <head>. Since it already has the
defer attribute it won't block loading. The defer attribute does preserve
ordering between scripts, so instead of the callback on load, it can set a
global variable on load, which is slightly simpler. Also, since it is required
to finish rendering the page, beginning its load earlier is better.

Remove generation and handling of sidebar-vars. Everything there can be computed
with information available in JS via other means.

Remove the extra_scripts fields of the Page template. They were only
used by source-script.js and source-files.js, which are now linked by the template
based on whether it is rendering a source page.

Remove the "other" wrapper in the sidebar. It was unnecessary.

r? @GuillaumeGomez

Demo: https://rustdoc.crud.net/jsha/defer-crates/std/index.html

@jsha jsha added T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. A-rustdoc-ui Area: Rustdoc UI (generated HTML) labels Jun 15, 2022
@rust-highfive
Copy link
Collaborator

Some changes occurred in HTML/CSS/JS.

cc @GuillaumeGomez,@Folyd,@jsha

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jun 15, 2022
@rust-log-analyzer

This comment has been minimized.

@GuillaumeGomez
Copy link
Member

Looks great, thanks! r=me with CI fixed.

@GuillaumeGomez
Copy link
Member

Oh actually, on source code pages, you don't need to load sidebar-items.js. Please only include it on documentation pages.

@rust-log-analyzer

This comment has been minimized.

@GuillaumeGomez
Copy link
Member

I think you need to update the Cargo.lock file (x.py build --stage 1 should be enough).

@GuillaumeGomez
Copy link
Member

Apart from that, the simplification of the code is greatly appreciated, thanks for it! :)

@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

Now that the "All Crates" dropdown is only rendered on the search results page,
there is no need to load crates.js on most pages. Load it only on crate pages.
Also, add the `defer` attribute so it does not block page rendering.

For sidebar-items.js, move the script tag to `<head>`. Since it already has the
defer attribute it won't block loading. The defer attribute does preserve
ordering between scripts, so instead of the callback on load, it can set a
global variable on load, which is slightly simpler. Also, since it is required
to finish rendering the page, beginning its load earlier is better.

Remove generation and handling of sidebar-vars. Everything there can be computed
with information available in JS via other means.

Remove the "other" wrapper in the sidebar. It was unnecessary.

Remove excess script fields
@jsha
Copy link
Contributor Author

jsha commented Jun 20, 2022

@bors r=GuillaumeGomez rollup

@bors
Copy link
Contributor

bors commented Jun 20, 2022

📌 Commit 27dcebe has been approved by GuillaumeGomez

@bors bors removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jun 20, 2022
@bors bors added the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label Jun 20, 2022
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Jun 21, 2022
Improve loading of crates.js and sidebar-items.js

Now that the "All Crates" dropdown is only rendered on the search results page,
there is no need to load crates.js on most pages. Load it only on crate pages.
Also, add the `defer` attribute so it does not block HTML parsing.

For sidebar-items.js, move the script tag to `<head>`. Since it already has the
defer attribute it won't block loading. The defer attribute does preserve
ordering between scripts, so instead of the callback on load, it can set a
global variable on load, which is slightly simpler. Also, since it is required
to finish rendering the page, beginning its load earlier is better.

Remove generation and handling of sidebar-vars. Everything there can be computed
with information available in JS via other means.

Remove the extra_scripts fields of the `Page` template. They were only
used by source-script.js and source-files.js, which are now linked by the template
based on whether it is rendering a source page.

Remove the "other" wrapper in the sidebar. It was unnecessary.

r? `@GuillaumeGomez`

Demo: https://rustdoc.crud.net/jsha/defer-crates/std/index.html
JohnTitor added a commit to JohnTitor/rust that referenced this pull request Jun 21, 2022
Improve loading of crates.js and sidebar-items.js

Now that the "All Crates" dropdown is only rendered on the search results page,
there is no need to load crates.js on most pages. Load it only on crate pages.
Also, add the `defer` attribute so it does not block HTML parsing.

For sidebar-items.js, move the script tag to `<head>`. Since it already has the
defer attribute it won't block loading. The defer attribute does preserve
ordering between scripts, so instead of the callback on load, it can set a
global variable on load, which is slightly simpler. Also, since it is required
to finish rendering the page, beginning its load earlier is better.

Remove generation and handling of sidebar-vars. Everything there can be computed
with information available in JS via other means.

Remove the extra_scripts fields of the `Page` template. They were only
used by source-script.js and source-files.js, which are now linked by the template
based on whether it is rendering a source page.

Remove the "other" wrapper in the sidebar. It was unnecessary.

r? ``@GuillaumeGomez``

Demo: https://rustdoc.crud.net/jsha/defer-crates/std/index.html
bors added a commit to rust-lang-ci/rust that referenced this pull request Jun 21, 2022
Rollup of 11 pull requests

Successful merges:

 - rust-lang#94033 (Improve docs for `is_running` to explain use case)
 - rust-lang#97269 (adjust transmute const stabilization version)
 - rust-lang#97805 (Add proper tracing spans to rustc_trait_selection::traits::error_reporting)
 - rust-lang#98022 (Fix erroneous span for borrowck error)
 - rust-lang#98124 (Improve loading of crates.js and sidebar-items.js)
 - rust-lang#98278 (Some token stream cleanups)
 - rust-lang#98306 (`try_fold_unevaluated` for infallible folders)
 - rust-lang#98313 (Remove lies in comments.)
 - rust-lang#98323 (:arrow_up: rust-analyzer)
 - rust-lang#98329 (Avoid an ICE and instead let the compiler report a useful error)
 - rust-lang#98330 (update ioslice docs to use shared slices)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 75f17ed into rust-lang:master Jun 21, 2022
@rustbot rustbot added this to the 1.63.0 milestone Jun 21, 2022
Folyd added a commit to huhu/rust-search-extension that referenced this pull request Jul 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-rustdoc-ui Area: Rustdoc UI (generated HTML) 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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants