rustdoc: more js cleanup by lolbinarycat · Pull Request #151386 · rust-lang/rust

lolbinarycat · 2026-01-19T23:13:22Z

Continuing the effort of removing @ts-expect-error from the codebase wherever possible, this time focusing on main.js. Found some oddities with register_type_impls, fixed most of them, but the one that I couldn't figure out is what's going on with sidebarTraitList. It's queried, then if there are any trait imps, unconditionally overwritten, then latter code assumes that one of these two things has initialized it, but it's not obvious why this would be the case, or if there's a reason this wasn't done in a more straightforwards way.

r? @GuillaumeGomez

rustbot · 2026-01-19T23:13:25Z

Some changes occurred in HTML/CSS/JS.

cc @GuillaumeGomez

GuillaumeGomez · 2026-01-19T23:22:20Z

src/librustdoc/html/static/js/storage.js

+    if (v !== null) {
+        return v;
+    }
+    throw "rustdoc var " + name + " is missing";


Suggested change

throw "rustdoc var " + name + " is missing";

throw `rustdoc var "${name}" is missing`;

GuillaumeGomez · 2026-01-19T23:23:41Z

src/librustdoc/html/static/js/main.js

        while (elem) {
-            if (elem.tagName === "DETAILS") {
-                // @ts-expect-error
+            if (elem instanceof HTMLDetailsElement) {


Although it's more correct, I find it so much less readable. ^^' (nothing to change here, just me sad that better code doesn't always look better)

GuillaumeGomez · 2026-01-19T23:24:28Z

src/librustdoc/html/static/js/main.js

        let methods = document.querySelector(".sidebar .block.method");
        let associatedTypes = document.querySelector(".sidebar .block.associatedtype");
        let associatedConstants = document.querySelector(".sidebar .block.associatedconstant");
+


nitpick: any reason for this new line?

Nope. will fix.

GuillaumeGomez · 2026-01-19T23:25:16Z

src/librustdoc/html/static/js/main.js

                }
            }
            const template = document.createElement("template");
+            // @ts-expect-error


What error is expected here btw?

string | 0 not assignable to string | null. this lead me to investigating TypeImpls generation and reading the code more thoroughly, which led me to realize only the second item in the array could be 0, thus the more complicated type definition.

GuillaumeGomez · 2026-01-19T23:26:01Z

src/librustdoc/html/static/js/main.js

+                a.href = `#${nonnull(template.content.querySelector(".impl")).id}`;
                // @ts-expect-error
-                a.href = `#${template.content.querySelector(".impl").id}`;
                a.textContent = traitName;


Why does this one has expect-error now?

It shouldn't, I think the intermediate commit got messed up by some rebasing.

Ah, wait, I misread the diff, this is also Type 'string | 0' is not assignable to type 'string | null'., but this time the error is somewhat legitimate (well, it's a limitation of typescript not realizing that the value of isTrait is linked to the type of traitName)

Can you add this as comment so future us will not have to search again for this explanation please?

yeah will do. i originally didn't because i hoped i could find a way to get rid of it easily.

GuillaumeGomez · 2026-01-19T23:27:09Z

src/librustdoc/html/static/js/main.js

                        mainContent.insertBefore(outputListHeader, trait_implementations_header);
                        mainContent.insertBefore(outputList, trait_implementations_header);
                    } else {
-                        const mainContent = nonnull(document.querySelector("#main-content"));


Nice catch. 👍

GuillaumeGomez · 2026-01-19T23:27:41Z

Apart from minor improvements, looks much better, thanks!

@ts-expect-error

This has two advantages: * removes a bunch of @ts-expect-error * typos of var names will cause type errors instead of runtime errors

GuillaumeGomez

Once the comment has been added to the code and the CI is green, r=me.

View changes since this review

lolbinarycat · 2026-01-28T17:12:30Z

Typing out "typescript doesn't understand this relation" made me wonder if I could make it understand, and the answer was yes, so...

Instead of adding a comment explaining the @ts-expect-error, I just removed it.

this allows TypeScript to understand the relation between isTrait and traitName's type, getting rid of a type error.

GuillaumeGomez · 2026-01-28T20:49:18Z

src/librustdoc/html/static/js/main.js

            const text = impList[0];
-            const isTrait = impList[1] !== 0;
            const traitName = impList[1];
+            const isTrait = typeof traitName === "string";


That's not the same check at all. Is there a reason why we go from integer comparison to checking it's a string? (lack of context on my side here)

impList[1] has type string | 0, so this is actually an equivalent check, though I could see how it could really not look like it without that context.

I see. JS is weird and we really abuse it sometimes. XD

GuillaumeGomez · 2026-02-13T23:00:18Z

Thanks!

@bors r+ rollup

rust-bors · 2026-02-13T23:00:22Z

📌 Commit bd1c36a has been approved by GuillaumeGomez

It is now in the queue for this repository.

@GuillaumeGomez

…=GuillaumeGomez rustdoc: more js cleanup Continuing the effort of removing `@ts-expect-error` from the codebase wherever possible, this time focusing on `main.js`. Found some oddities with `register_type_impls`, fixed most of them, but the one that I couldn't figure out is what's going on with `sidebarTraitList`. It's queried, then if there are any trait imps, unconditionally overwritten, then latter code assumes that one of these two things has initialized it, but it's not obvious why this would be the case, or if there's a reason this wasn't done in a more straightforwards way. r? @GuillaumeGomez

Rollup of 11 pull requests Successful merges: - #151998 (Set hidden visibility on naked functions in compiler-builtins) - #149460 (rustdoc: sort stable items first) - #152076 (Feed `ErrorGuaranteed` from late lifetime resolution errors through to bound variable resolution) - #152471 (improve associated-type suggestions from bounds) - #152573 (move `escape_symbol_name` to `cg_ssa`) - #152594 (c-variadic: implement `va_arg` for `wasm64`) - #151386 (rustdoc: more js cleanup) - #152567 (nix-dev-shell: fix a typo) - #152568 (Port `#[lang]` and `#[panic_handler]` to the new attribute parsers) - #152575 (layout_of unexpected rigid alias delayed bug) - #152587 (A couple of tiny polonius things)

rustbot assigned GuillaumeGomez Jan 19, 2026

GuillaumeGomez reviewed Jan 19, 2026

View reviewed changes

lolbinarycat force-pushed the rustdoc-ts-cleanup branch 2 times, most recently from 197be7f to 519c0d9 Compare January 19, 2026 23:39

lolbinarycat added 6 commits January 19, 2026 17:39

rustdoc: change getVar signature

ca5a254

This has two advantages: * removes a bunch of @ts-expect-error * typos of var names will cause type errors instead of runtime errors

rustdoc(storage.js): add comment explaining use of @ts-ignore

b7ba80d

rustdoc(main.js): use instanceof instead of tagName where applicable

760d886

rustdoc(main.js): use nonnull to clear up type errors

38d7b53

rustdoc: make TypeImpls more specific.

3b4de4b

rustdoc: remove redundant mainContent variable

519c0d9

GuillaumeGomez approved these changes Jan 20, 2026

View reviewed changes

rustdoc(main.js): use typeof in register_type_impls for isTrait

bd1c36a

this allows TypeScript to understand the relation between isTrait and traitName's type, getting rid of a type error.

GuillaumeGomez reviewed Jan 28, 2026

View reviewed changes

GuillaumeGomez approved these changes Feb 13, 2026

View reviewed changes

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 Feb 13, 2026

jhpratt mentioned this pull request Feb 14, 2026

Rollup of 11 pull requests #152602

Open

	throw "rustdoc var " + name + " is missing";
	throw `rustdoc var "${name}" is missing`;

Uh oh!

Conversation

lolbinarycat commented Jan 19, 2026

Uh oh!

rustbot commented Jan 19, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

GuillaumeGomez commented Jan 19, 2026

Uh oh!

GuillaumeGomez left a comment • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lolbinarycat commented Jan 28, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

GuillaumeGomez commented Feb 13, 2026

Uh oh!

rust-bors bot commented Feb 13, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

GuillaumeGomez left a comment •

edited by rustbot

Loading