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

rustdoc: add nobuild typescript checking to our JS #136161

Merged
merged 1 commit into from
Jan 29, 2025

Conversation

notriddle
Copy link
Contributor

By nobuild, I mean that the type annotations are all in comments, not in the "native" typescript syntax. This is a bit uglier, but it lets you rapid-prototype without tsc, works with all the native browser debugging tools, and keeps Node out of Rust's bootstrap chain.

This pull request mostly just adds ts-ignore annotations and type declarations. To actually take good advantage of typescript, we'll want to "burn down" this pile of unsafe code until we eventually have a version with almost none of these.

This PR also adds tsc to the mingw-check Dockerfile, so that it can't fall out of date like the Closure annotations did.

https://rust-lang.zulipchat.com/#narrow/channel/266220-t-rustdoc/topic/typescript

r? @GuillaumeGomez @lolbinarycat

@rustbot rustbot added A-testsuite Area: The testsuite used to check the correctness of rustc S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-infra Relevant to the infrastructure team, which will review and decide on the PR/issue. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. labels Jan 27, 2025
@rustbot
Copy link
Collaborator

rustbot commented Jan 27, 2025

Some changes occurred in HTML/CSS/JS.

cc @GuillaumeGomez, @jsha

@rust-log-analyzer

This comment has been minimized.

@uellenberg
Copy link
Contributor

It may be better to use @ts-expect-error in place of @ts-ignore. That way, if a change somewhere fixes an error down the line, TypeScript will detect and report it (since expect-error requires that the next line creates an error).

@rust-log-analyzer

This comment has been minimized.

@notriddle notriddle force-pushed the typescript branch 2 times, most recently from 68d7aac to 7e3f583 Compare January 28, 2025 01:53
By nobuild, I mean that the type annotations are all in comments,
not in the "native" typescript syntax. This is a bit uglier,
but it lets you rapid-prototype without tsc, works with all
the native browser debugging tools, and keeps Node out of Rust's
bootstrap chain.

This pull request mostly just adds ts-ignore annotations
and type declarations. To actually take good advantage of
typescript, we'll want to "burn down" this pile of unsafe code
until we eventually have a version with almost none of these.

This PR also adds tsc to the mingw-check Dockerfile, so that
it can't fall out of date like the Closure annotations did.

https://rust-lang.zulipchat.com/#narrow/channel/266220-t-rustdoc/topic/typescript
@GuillaumeGomez
Copy link
Member

This is impressive. Thanks a lot for this improvement! I particularly appreciate that we don't require TS to build rustdoc, only to check its JS like we do with eslint.

@bors r+

@bors
Copy link
Contributor

bors commented Jan 28, 2025

📌 Commit d94b64d has been approved by GuillaumeGomez

It is now in the queue for this repository.

@bors 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-review Status: Awaiting review from the assignee but also interested parties. labels Jan 28, 2025
bors added a commit to rust-lang-ci/rust that referenced this pull request Jan 29, 2025
Rollup of 9 pull requests

Successful merges:

 - rust-lang#136121 (Deduplicate operand creation between scalars, non-scalars and string patterns)
 - rust-lang#136134 (Fix SIMD codegen tests on LLVM 20)
 - rust-lang#136153 (Locate asan-odr-win with other sanitizer tests)
 - rust-lang#136161 (rustdoc: add nobuild typescript checking to our JS)
 - rust-lang#136166 (interpret: is_alloc_live: check global allocs last)
 - rust-lang#136168 (GCI: Don't try to eval / collect mono items inside overly generic free const items)
 - rust-lang#136170 (Reject unsound toggling of Arm atomics-32 target feature)
 - rust-lang#136176 (Render pattern types nicely in mir dumps)
 - rust-lang#136186 (uefi: process: Fix args)

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit to rust-lang-ci/rust that referenced this pull request Jan 29, 2025
Rollup of 9 pull requests

Successful merges:

 - rust-lang#136121 (Deduplicate operand creation between scalars, non-scalars and string patterns)
 - rust-lang#136134 (Fix SIMD codegen tests on LLVM 20)
 - rust-lang#136153 (Locate asan-odr-win with other sanitizer tests)
 - rust-lang#136161 (rustdoc: add nobuild typescript checking to our JS)
 - rust-lang#136166 (interpret: is_alloc_live: check global allocs last)
 - rust-lang#136168 (GCI: Don't try to eval / collect mono items inside overly generic free const items)
 - rust-lang#136170 (Reject unsound toggling of Arm atomics-32 target feature)
 - rust-lang#136176 (Render pattern types nicely in mir dumps)
 - rust-lang#136186 (uefi: process: Fix args)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 3cc6ea2 into rust-lang:master Jan 29, 2025
6 checks passed
@rustbot rustbot added this to the 1.86.0 milestone Jan 29, 2025
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Jan 29, 2025
Rollup merge of rust-lang#136161 - notriddle:typescript, r=GuillaumeGomez

rustdoc: add nobuild typescript checking to our JS

By nobuild, I mean that the type annotations are all [in comments], not in the "native" typescript syntax. This is a bit uglier, but it lets you rapid-prototype without tsc, works with all the native browser debugging tools, and keeps Node out of Rust's bootstrap chain.

[in comments]: https://news.ycombinator.com/item?id=35892250

This pull request mostly just adds ts-ignore annotations and type declarations. To actually take good advantage of typescript, we'll want to "burn down" this pile of unsafe code until we eventually have a version with almost none of these.

This PR also adds tsc to the mingw-check Dockerfile, so that it can't fall out of date like the Closure annotations did.

https://rust-lang.zulipchat.com/#narrow/channel/266220-t-rustdoc/topic/typescript

r? `@GuillaumeGomez` `@lolbinarycat`
@fmease
Copy link
Member

fmease commented Jan 29, 2025

FYI: I'm suspecting this caused slight perf regressions. See #136227 (comment). I'll let you be the judge of that :)

@notriddle notriddle deleted the typescript branch January 29, 2025 16:12
bors added a commit to rust-lang-ci/rust that referenced this pull request Jan 29, 2025
rustdoc: run css and html minifier at build instead of runtime

This way, adding a bunch of comments to the JS files won't make rustdoc slower.

Meant to address rust-lang#136161 (comment)
@fmease
Copy link
Member

fmease commented Jan 30, 2025

the type annotations are all in comments, not in the "native" typescript syntax

Disclaimer: I haven't played around with the newly annotated JS files, so I might be missing some obvious things! Is it actually a hard requirement to use JSDoc's / Closure's type syntax? As far as I understand and have tested, tsc is perfectly fine with using (most) TS type syntax inside JSDoc annotations.

TS's docs call this out (source):

You can use most JSDoc type syntax and any TypeScript syntax, from the most basic like string to the most advanced, like conditional types. [emphasis mine]

Or did you intend to offer people the chance to typecheck the project locally with the Closure compiler instead (which seems to be the only tool other than tsc that can consume and check JSDoc type annotations)?

I guess third party tools (like editors) might suffer from using TS syntax instead. Of course, VS Code's integrated JS/TS language server supports this out of the box.

@fmease
Copy link
Member

fmease commented Jan 30, 2025

I was just wondering about that while reviewing #136263 where I saw the JSDoc-style function(): any.

@notriddle
Copy link
Contributor Author

Or did you intend to offer people the chance to typecheck the project locally with the Closure compiler instead (which seems to be the only tool other than tsc that can consume and check JSDoc type annotations)?

No, you can use all the typescript-specific syntax you want as long as it's in comments (or in metadata files that aren't loaded by the browser). I don't see any point in trying to support multiple type checkers.

jhpratt added a commit to jhpratt/rust that referenced this pull request Feb 4, 2025
…=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
fmease added a commit to fmease/rust that referenced this pull request Feb 5, 2025
…=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
bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 5, 2025
…illaumeGomez

rustdoc: run css and html minifier at build instead of runtime

This way, adding a bunch of comments to the JS files won't make rustdoc slower.

Meant to address rust-lang#136161 (comment)
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Feb 5, 2025
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
bors added a commit to rust-lang-ci/rust that referenced this pull request Feb 5, 2025
…illaumeGomez

rustdoc: run css and html minifier at build instead of runtime

This way, adding a bunch of comments to the JS files won't make rustdoc slower.

Meant to address rust-lang#136161 (comment)
github-actions bot pushed a commit to rust-lang/rustc-dev-guide that referenced this pull request Feb 6, 2025
rustdoc: run css and html minifier at build instead of runtime

This way, adding a bunch of comments to the JS files won't make rustdoc slower.

Meant to address rust-lang/rust#136161 (comment)
github-actions bot pushed a commit to rust-lang/miri that referenced this pull request Feb 6, 2025
rustdoc: run css and html minifier at build instead of runtime

This way, adding a bunch of comments to the JS files won't make rustdoc slower.

Meant to address rust-lang/rust#136161 (comment)
@marxin
Copy link
Contributor

marxin commented Feb 6, 2025

@notriddle

Can you please update rustc-dev-guide where a new borked link has been detected?

error: Server returned 404 Not Found for https://github.com/rust-lang/rust/blob/master/src/librustdoc/html/static/js/externs.js
    ┌─ rustdoc-internals/search.md:482:1
    │
482 │ [`externs.js`](https://github.com/rust-lang/rust/blob/master/src/librustdoc/html/static/js/externs.js).
    │ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Server returned 404 Not Found for https://github.com/rust-lang/rust/blob/master/src/librustdoc/html/static/js/externs.js

@notriddle
Copy link
Contributor Author

lnicola pushed a commit to lnicola/rust-analyzer that referenced this pull request Feb 10, 2025
rustdoc: run css and html minifier at build instead of runtime

This way, adding a bunch of comments to the JS files won't make rustdoc slower.

Meant to address rust-lang/rust#136161 (comment)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-testsuite Area: The testsuite used to check the correctness of rustc S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-infra Relevant to the infrastructure team, which will review and decide on the PR/issue. 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.

8 participants