Skip to content

rustdoc js: Even more typechecking improvments #145742

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

Merged
merged 7 commits into from
Aug 23, 2025
Merged

rustdoc js: Even more typechecking improvments #145742

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
5d28ce4
typecheck window.searchIndex
lolbinarycat Aug 22, 2025
4b73a7e
typecheck window.rr_
lolbinarycat Aug 22, 2025
6c22ef5
typecheck window.NOTABLE_TRAITS
lolbinarycat Aug 22, 2025
dfec2bb
typecheck window.CURRENT_TOOLTIP_ELEMENT
lolbinarycat Aug 22, 2025
cb572cb
typecheck tooltipBlurHandler
lolbinarycat Aug 22, 2025
4a2d6df
typecheck: add nonnull around element known to exist
lolbinarycat Aug 22, 2025
771c641
main.js: only call window.rustdocToggleSrcSidebar if it exists
lolbinarycat Aug 22, 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
50 changes: 10 additions & 40 deletions src/librustdoc/html/static/js/main.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -407,9 +407,7 @@ function preLoadCss(cssUrl) {
function loadSearch() {
if (!searchLoaded) {
searchLoaded = true;
// @ts-expect-error
window.rr_ = data => {
// @ts-expect-error
window.searchIndex = data;
};
if (!window.StringdexOnload) {
Expand Down Expand Up @@ -1277,13 +1275,11 @@ function preLoadCss(cssUrl) {
}

window.addEventListener("resize", () => {
// @ts-expect-error
if (window.CURRENT_TOOLTIP_ELEMENT) {
// As a workaround to the behavior of `contains: layout` used in doc togglers,
// tooltip popovers are positioned using javascript.
//
// This means when the window is resized, we need to redo the layout.
// @ts-expect-error
const base = window.CURRENT_TOOLTIP_ELEMENT.TOOLTIP_BASE;
const force_visible = base.TOOLTIP_FORCE_VISIBLE;
hideTooltip(false);
Expand Down Expand Up @@ -1329,26 +1325,25 @@ function preLoadCss(cssUrl) {
*/
function showTooltip(e) {
const notable_ty = e.getAttribute("data-notable-ty");
// @ts-expect-error
if (!window.NOTABLE_TRAITS && notable_ty) {
const data = document.getElementById("notable-traits-data");
if (data) {
// @ts-expect-error
window.NOTABLE_TRAITS = JSON.parse(data.innerText);
} else {
throw new Error("showTooltip() called with notable without any notable traits!");
}
}
// Make this function idempotent. If the tooltip is already shown, avoid doing extra work
// and leave it alone.
// @ts-expect-error
if (window.CURRENT_TOOLTIP_ELEMENT && window.CURRENT_TOOLTIP_ELEMENT.TOOLTIP_BASE === e) {
// @ts-expect-error
clearTooltipHoverTimeout(window.CURRENT_TOOLTIP_ELEMENT);
return;
}
window.hideAllModals(false);
const wrapper = document.createElement("div");
// use Object.assign to make sure the object has the correct type
// with all of the correct fields before it is assigned to a variable,
// as typescript has no way to change the type of a variable once it is initialized.
const wrapper = Object.assign(document.createElement("div"), {TOOLTIP_BASE: e});
Copy link
Member

Choose a reason for hiding this comment

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

Is there an advantage assigning a field this way? It's less readable that writing wrapper.TOOLTIP_BASE = e; (which should have been how we should have written it in the first place).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

the way i'm writing it results in a variable of type HTMLDivElement & { TOOLTIP_BASE: HTMLElement }, while what you're reccomendeding would result in a variable of type HTMLDivElement, and then you would try assigning to a field that does not exist on that type, which results in a type error.

unfortunately there's no way to have an expression that changes the type of a variable that already exists, as handy as that would be (it would be especially helpful for functions that finish initializing partially initialized objects).

I don't know of any other way to avoid a type error besides a cast, which I would quite like to avoid.

Copy link
Member

Choose a reason for hiding this comment

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

When a tool forces to write less good code, it's generally not a great sign... Please add a comment on why it's done this way then. I'll review one last time once done and then merge.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added a comment.

if (notable_ty) {
wrapper.innerHTML = "<div class=\"content\">" +
// @ts-expect-error
Expand Down Expand Up @@ -1394,11 +1389,7 @@ function preLoadCss(cssUrl) {
);
}
wrapper.style.visibility = "";
// @ts-expect-error
window.CURRENT_TOOLTIP_ELEMENT = wrapper;
// @ts-expect-error
window.CURRENT_TOOLTIP_ELEMENT.TOOLTIP_BASE = e;
// @ts-expect-error
clearTooltipHoverTimeout(window.CURRENT_TOOLTIP_ELEMENT);
wrapper.onpointerenter = ev => {
// If this is a synthetic touch event, ignore it. A click event will be along shortly.
Expand Down Expand Up @@ -1433,19 +1424,15 @@ function preLoadCss(cssUrl) {
*/
function setTooltipHoverTimeout(element, show) {
clearTooltipHoverTimeout(element);
// @ts-expect-error
if (!show && !window.CURRENT_TOOLTIP_ELEMENT) {
// To "hide" an already hidden element, just cancel its timeout.
return;
}
// @ts-expect-error
if (show && window.CURRENT_TOOLTIP_ELEMENT) {
// To "show" an already visible element, just cancel its timeout.
return;
}
// @ts-expect-error
if (window.CURRENT_TOOLTIP_ELEMENT &&
// @ts-expect-error
window.CURRENT_TOOLTIP_ELEMENT.TOOLTIP_BASE !== element) {
// Don't do anything if another tooltip is already visible.
return;
Expand All @@ -1468,24 +1455,20 @@ function preLoadCss(cssUrl) {
*/
function clearTooltipHoverTimeout(element) {
if (element.TOOLTIP_HOVER_TIMEOUT !== undefined) {
// @ts-expect-error
removeClass(window.CURRENT_TOOLTIP_ELEMENT, "fade-out");
clearTimeout(element.TOOLTIP_HOVER_TIMEOUT);
delete element.TOOLTIP_HOVER_TIMEOUT;
}
}

// @ts-expect-error
/**
* @param {Event & { relatedTarget: Node }} event
*/
function tooltipBlurHandler(event) {
// @ts-expect-error
if (window.CURRENT_TOOLTIP_ELEMENT &&
// @ts-expect-error
!window.CURRENT_TOOLTIP_ELEMENT.contains(document.activeElement) &&
// @ts-expect-error
!window.CURRENT_TOOLTIP_ELEMENT.contains(event.relatedTarget) &&
// @ts-expect-error
!window.CURRENT_TOOLTIP_ELEMENT.TOOLTIP_BASE.contains(document.activeElement) &&
// @ts-expect-error
!window.CURRENT_TOOLTIP_ELEMENT.TOOLTIP_BASE.contains(event.relatedTarget)
) {
// Work around a difference in the focus behaviour between Firefox, Chrome, and Safari.
Expand All @@ -1507,30 +1490,22 @@ function preLoadCss(cssUrl) {
* If set to `false`, leave keyboard focus alone.
*/
function hideTooltip(focus) {
// @ts-expect-error
if (window.CURRENT_TOOLTIP_ELEMENT) {
// @ts-expect-error
if (window.CURRENT_TOOLTIP_ELEMENT.TOOLTIP_BASE.TOOLTIP_FORCE_VISIBLE) {
if (focus) {
// @ts-expect-error
window.CURRENT_TOOLTIP_ELEMENT.TOOLTIP_BASE.focus();
}
// @ts-expect-error
window.CURRENT_TOOLTIP_ELEMENT.TOOLTIP_BASE.TOOLTIP_FORCE_VISIBLE = false;
}
// @ts-expect-error
document.body.removeChild(window.CURRENT_TOOLTIP_ELEMENT);
// @ts-expect-error
clearTooltipHoverTimeout(window.CURRENT_TOOLTIP_ELEMENT);
// @ts-expect-error
window.CURRENT_TOOLTIP_ELEMENT = null;
window.CURRENT_TOOLTIP_ELEMENT = undefined;
}
}

onEachLazy(document.getElementsByClassName("tooltip"), e => {
e.onclick = () => {
e.TOOLTIP_FORCE_VISIBLE = e.TOOLTIP_FORCE_VISIBLE ? false : true;
// @ts-expect-error
if (window.CURRENT_TOOLTIP_ELEMENT && !e.TOOLTIP_FORCE_VISIBLE) {
hideTooltip(true);
} else {
Expand Down Expand Up @@ -1566,9 +1541,7 @@ function preLoadCss(cssUrl) {
if (ev.pointerType !== "mouse") {
return;
}
// @ts-expect-error
if (!e.TOOLTIP_FORCE_VISIBLE && window.CURRENT_TOOLTIP_ELEMENT &&
// @ts-expect-error
!window.CURRENT_TOOLTIP_ELEMENT.contains(ev.relatedTarget)) {
// Tooltip pointer leave gesture:
//
Expand Down Expand Up @@ -1601,7 +1574,6 @@ function preLoadCss(cssUrl) {
// * https://www.nngroup.com/articles/tooltip-guidelines/
// * https://bjk5.com/post/44698559168/breaking-down-amazons-mega-dropdown
setTooltipHoverTimeout(e, false);
// @ts-expect-error
addClass(window.CURRENT_TOOLTIP_ELEMENT, "fade-out");
}
};
Expand Down Expand Up @@ -1707,8 +1679,7 @@ function preLoadCss(cssUrl) {
if (isHelpPage) {
const help_section = document.createElement("section");
help_section.appendChild(container);
// @ts-expect-error
document.getElementById("main-content").appendChild(help_section);
nonnull(document.getElementById("main-content")).appendChild(help_section);
} else {
onEachLazy(document.getElementsByClassName("help-menu"), menu => {
if (menu.offsetWidth !== 0) {
Expand Down Expand Up @@ -1854,8 +1825,7 @@ function preLoadCss(cssUrl) {
sidebarButton.addEventListener("click", e => {
removeClass(document.documentElement, "hide-sidebar");
updateLocalStorage("hide-sidebar", "false");
if (document.querySelector(".rustdoc.src")) {
// @ts-expect-error
if (window.rustdocToggleSrcSidebar) {
window.rustdocToggleSrcSidebar();
}
e.preventDefault();
Expand Down
7 changes: 7 additions & 0 deletions src/librustdoc/html/static/js/rustdoc.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,9 @@ declare global {
currentTheme: HTMLLinkElement|null;
/** Generated in `render/context.rs` */
SIDEBAR_ITEMS?: { [key: string]: string[] };
/** Notable trait data */
NOTABLE_TRAITS?: { [key: string]: string };
CURRENT_TOOLTIP_ELEMENT?: HTMLElement & { TOOLTIP_BASE: HTMLElement };
/** Used by the popover tooltip code. */
RUSTDOC_TOOLTIP_HOVER_MS: number;
/** Used by the popover tooltip code. */
Expand Down Expand Up @@ -93,6 +96,10 @@ declare global {
pending_type_impls?: rustdoc.TypeImpls,
rustdoc_add_line_numbers_to_examples?: function(),
rustdoc_remove_line_numbers_from_examples?: function(),
/** JSON-encoded raw search index */
searchIndex: string,
/** Used in search index shards in order to load data into the in-memory database */
rr_: function(string),
}
interface HTMLElement {
/** Used by the popover tooltip code. */
Expand Down
2 changes: 0 additions & 2 deletions src/librustdoc/html/static/js/search.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5244,9 +5244,7 @@ if (typeof window !== "undefined") {
// search.index/root is loaded by main.js, so
// this script doesn't need to launch it, but
// must pick it up
// @ts-ignore
if (window.searchIndex) {
// @ts-ignore
window.rr_(window.searchIndex);
}
},
Expand Down
2 changes: 1 addition & 1 deletion src/librustdoc/html/static/js/storage.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ function addClass(elem, className) {
* Remove a class from a DOM Element. If `elem` is null,
* does nothing. This function is idempotent.
*
* @param {Element|null} elem
* @param {Element|null|undefined} elem
* @param {string} className
*/
// eslint-disable-next-line no-unused-vars
Expand Down
Loading