rustdoc: clean up a bunch of ts-expected-error declarations in main by notriddle · Pull Request #136263 · rust-lang/rust

notriddle · 2025-01-29T20:17:53Z

This mostly consists of handling potentially-null input and adding more global functions to the list of globals.

Follow-up for #136161

rustbot · 2025-01-29T20:18:03Z

rustbot has assigned @fmease.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

rustbot · 2025-01-29T20:18:05Z

Some changes occurred in HTML/CSS/JS.

cc @GuillaumeGomez, @jsha

fmease

Nice, thanks! Two questions, r=me when resolved in one way or another

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

fmease · 2025-02-01T01:59:13Z

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

-    if (!resizer || !sidebar) {
+    // If this page has no sidebar at all, bail out.
+    if (!(resizer instanceof HTMLElement) || !(sidebar instanceof HTMLElement)) {
        return;


Why not /** @type {Element | null} */ for both resizer and sidebar and adding the check !resizer || !sidebar to the guarded return? This would eliminate the two new //@ts-expect-errors, wouldn't it?

(And it would eliminate the seeming tautology @type HTMLElement and !(resizer instanceof HTMLElement). I'm slightly surprised that querySelector doesn't unconditionally return Element if the param is string but I guess that's convenience over safety (ofc in practice none of this matters lol)?)

Unfortunately, this code uses offsetTop, which is defined on HTMLElement, not Element.

And querySelector returns Element.

And querySelector returns Element.

As far as I can tell, (the relevant overload of) querySelector returns any caller-provided E that extends Element which allows you to instantiate it with HTMLElement -- if it was Element you wouldn't've been able to cast it to HTMLElement which is more specific than Element.

Unfortunately, this code uses offsetTop, which is defined on HTMLElement, not Element.

Yes, resizer and sidebar would start out as Elements but due to the guarded returns of the form if (!(… instanceof HTMLElement)) { return; } the type is automatically narrowed to HTMLElement following the if statement as part of flow-sensitive typing / control-flow based type analysis which should permit you to access offsetTop on them.

// lib.dom.d.ts querySelector<E extends Element = Element>(selectors: string): E | null;

You're right. I was misunderstanding it. I admit, I'm not actually that familiar with TypeScript, beyond intro-level docs.

lolbinarycat · 2025-02-01T03:38:29Z

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

-        // @ts-expect-error
-        getSettingsButton().onclick = event => {
+    const settingsButton = getSettingsButton();
+    if (settingsButton) {


In the unlikely event that the settings button cannot be located, I think it would be better to fail-fast, rather than silently ignore it, no?

Even if we went ahead and let it crash, it would still just drop an error in the browser console, probably never to be noticed by anyone.

Fair, I suppose.

fmease · 2025-02-02T02:23:56Z

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

    // the minimum sidebar size is rather uncomfortable, and it must pass
    // through that size when using the shrink-to-nothing gesture.
-    function hideSidebar() {
+    const hideSidebar = function hideSidebar() {


Suggested change

const hideSidebar = function hideSidebar() {

const hideSidebar = function() {

Similarly for all other occurrences of this kind of change.

I guess this is just drive-by cleanup?

I guess this is just drive-by cleanup?

No, it prevents the function definitions from being hoisted above the null check.

fmease

r=me with new nitpick addressed and with the three last commits squashed

This mostly consists of handling potentially-null input and adding more global functions to the list of globals.

notriddle · 2025-02-03T18:05:19Z

@bors r=fmease rollup

bors · 2025-02-03T18:05:22Z

📌 Commit 2ea95f8 has been approved by fmease

It is now in the queue for this repository.

…=fmease rustdoc: clean up a bunch of ts-expected-error declarations in main This mostly consists of handling potentially-null input and adding more global functions to the list of globals. Follow-up for rust-lang#136161

Rollup of 9 pull requests Successful merges: - rust-lang#128045 (#[contracts::requires(...)] + #[contracts::ensures(...)]) - rust-lang#136263 (rustdoc: clean up a bunch of ts-expected-error declarations in main) - rust-lang#136375 (cg_llvm: Replace some DIBuilder wrappers with LLVM-C API bindings (part 1)) - rust-lang#136392 (bootstrap: add wrapper macros for `feature = "tracing"`-gated `tracing` macros) - rust-lang#136405 (rustdoc-book: Clean up section on `--output-format`) - rust-lang#136497 (Report generic mismatches when calling bodyless trait functions) - rust-lang#136502 (Mark `std::fmt::from_fn` as `#[must_use]`) - rust-lang#136509 (Add tests for nested macro_rules edition behavior) - rust-lang#136526 (mir_build: Rename `thir::cx::Cx` to `ThirBuildCx` and remove `UserAnnotatedTyHelpers`) r? `@ghost` `@rustbot` modify labels: rollup

Rollup of 8 pull requests Successful merges: - rust-lang#128045 (#[contracts::requires(...)] + #[contracts::ensures(...)]) - rust-lang#136263 (rustdoc: clean up a bunch of ts-expected-error declarations in main) - rust-lang#136375 (cg_llvm: Replace some DIBuilder wrappers with LLVM-C API bindings (part 1)) - rust-lang#136392 (bootstrap: add wrapper macros for `feature = "tracing"`-gated `tracing` macros) - rust-lang#136396 (rustdoc-json-types: Document that crate name isn't package name.) - rust-lang#136405 (rustdoc-book: Clean up section on `--output-format`) - rust-lang#136502 (Mark `std::fmt::from_fn` as `#[must_use]`) - rust-lang#136509 (Add tests for nested macro_rules edition behavior) r? `@ghost` `@rustbot` modify labels: rollup

Rollup merge of rust-lang#136263 - notriddle:notriddle/typescript2, r=fmease rustdoc: clean up a bunch of ts-expected-error declarations in main This mostly consists of handling potentially-null input and adding more global functions to the list of globals. Follow-up for rust-lang#136161

rustbot assigned fmease Jan 29, 2025

rustbot added 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. labels Jan 29, 2025

This comment has been minimized.

Sign in to view

notriddle force-pushed the notriddle/typescript2 branch 2 times, most recently from 1a99bc5 to cf3ce0a Compare January 29, 2025 22:30

This comment has been minimized.

Sign in to view

notriddle force-pushed the notriddle/typescript2 branch from cf3ce0a to db13f16 Compare January 30, 2025 00:16

This comment has been minimized.

Sign in to view

fmease added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jan 30, 2025

fmease mentioned this pull request Jan 30, 2025

rustdoc: add nobuild typescript checking to our JS #136161

Merged

notriddle force-pushed the notriddle/typescript2 branch from db13f16 to 85f7423 Compare January 30, 2025 20:23

fmease approved these changes Feb 1, 2025

View reviewed changes

lolbinarycat reviewed Feb 1, 2025

View reviewed changes

lolbinarycat mentioned this pull request Feb 1, 2025

remove a bunch of @js-expect-error and generally make rustdoc js more typechecked. #136372

Closed

fmease reviewed Feb 2, 2025

View reviewed changes

rustdoc: clean up a bunch of ts-expected-error declarations in main

2ea95f8

This mostly consists of handling potentially-null input and adding more global functions to the list of globals.

notriddle force-pushed the notriddle/typescript2 branch from 85aca23 to 2ea95f8 Compare February 3, 2025 17:06

bors 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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Feb 3, 2025

jhpratt mentioned this pull request Feb 4, 2025

Rollup of 9 pull requests #136531

Closed

fmease mentioned this pull request Feb 5, 2025

Rollup of 8 pull requests #136564

Merged

bors merged commit 207777b into rust-lang:master Feb 5, 2025
6 checks passed

rustbot added this to the 1.86.0 milestone Feb 5, 2025

notriddle deleted the notriddle/typescript2 branch February 5, 2025 17:11

	const hideSidebar = function hideSidebar() {
	const hideSidebar = function() {

Uh oh!

Conversation

notriddle commented Jan 29, 2025

Uh oh!

rustbot commented Jan 29, 2025

Uh oh!

rustbot commented Jan 29, 2025

Uh oh!

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

fmease left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

fmease Feb 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

fmease Feb 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

fmease Feb 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

fmease left a comment

Choose a reason for hiding this comment

Uh oh!

notriddle commented Feb 3, 2025

Uh oh!

bors commented Feb 3, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

fmease Feb 1, 2025 •

edited

Loading

fmease Feb 1, 2025 •

edited

Loading

fmease Feb 2, 2025 •

edited

Loading