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: replace var with const and let in JS #93058

Closed
jsha opened this issue Jan 19, 2022 · 10 comments
Closed

rustdoc: replace var with const and let in JS #93058

jsha opened this issue Jan 19, 2022 · 10 comments
Labels
A-rustdoc-js Area: Rustdoc's JS front-end T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.

Comments

@jsha
Copy link
Contributor

jsha commented Jan 19, 2022

Now that rustdoc is using ES6 idioms, we should replace all var declarations with const and let. This style guideline explains why: https://github.com/airbnb/javascript#references. Once that's done, we should turn on the lint checks that enforce it.

I think there are a few places (at least in search.js) where we rely on the surprising property of var declarations called "var hoisting". In those cases we may to move some declarations to the top of their function at the same time we change them to let.

@jsha jsha added T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. A-rustdoc-js Area: Rustdoc's JS front-end labels Jan 19, 2022
@GuillaumeGomez
Copy link
Member

I'll do it once I'm done with #90630.

@the8472
Copy link
Member

the8472 commented Apr 24, 2022

Shouldn't the JS also be changed to strict mode to ensure that all variables actually are declared via keywords?

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Apr 25, 2022
Switch JS code to ES6

Considering it's already quite big, I'll do the remaining files in another PR.

Part of rust-lang#93058.

r? `@notriddle`
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this issue Apr 25, 2022
Switch JS code to ES6

Considering it's already quite big, I'll do the remaining files in another PR.

Part of rust-lang#93058.

r? ``@notriddle``
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this issue Apr 25, 2022
Switch JS code to ES6

Considering it's already quite big, I'll do the remaining files in another PR.

Part of rust-lang#93058.

r? ```@notriddle```
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this issue Apr 25, 2022
Switch JS code to ES6

Considering it's already quite big, I'll do the remaining files in another PR.

Part of rust-lang#93058.

r? ````@notriddle````
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this issue Apr 26, 2022
Switch JS code to ES6

Considering it's already quite big, I'll do the remaining files in another PR.

Part of rust-lang#93058.

r? `@notriddle`
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this issue Apr 26, 2022
Switch JS code to ES6

Considering it's already quite big, I'll do the remaining files in another PR.

Part of rust-lang#93058.

r? ``@notriddle``
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this issue Apr 29, 2022
Switch JS code to ES6 - part 2

Part of rust-lang#93058.

It's based on rust-lang#96361 so it needs to wait for it to be merged first.

r? `@notriddle`
@GuillaumeGomez
Copy link
Member

GuillaumeGomez commented Apr 30, 2022

@Folyd: So a few things remain to be done:

  • Move callback to the () => {} syntax.
  • Use "strict" as suggested by @the8472
  • Add a lot more eslint lints.

Do you want to divide the work with me? :)

EDIT: sorry @fmease for the invalid ping ^^'

@Folyd
Copy link
Contributor

Folyd commented May 1, 2022

@Folyd: So a few things remain to be done:

  • Move callback to the () => {} syntax.
  • Use "strict" as suggested by @the8472
  • Add a lot more eslint lints.

Do you want to divide the work with me? :)

EDIT: sorry @fmease for the invalid ping ^^'

Sure, I'd love to. 😸

@GuillaumeGomez
Copy link
Member

Then pick one thing and go ahead. :)

@Folyd
Copy link
Contributor

Folyd commented May 1, 2022

Ok, I pick this one.

Move callback to the () => {} syntax.

@GuillaumeGomez
Copy link
Member

Then I'll do "use strict" next.

JohnTitor added a commit to JohnTitor/rust that referenced this issue May 5, 2022
Move callback to the () => {} syntax.

Part of rust-lang#93058.

r? `@GuillaumeGomez`
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue May 5, 2022
…triddle

Use "strict" mode in JS scripts

Part of rust-lang#93058.

r? `@notriddle`
@GuillaumeGomez
Copy link
Member

@Folyd Now remains adding more eslint lints. Let's go one at a time. What do you think? If you're interested, just write down which one you're adding here so that we don't conflict. ;)

@Folyd
Copy link
Contributor

Folyd commented May 7, 2022

@Folyd Now remains adding more eslint lints. Let's go one at a time. What do you think? If you're interested, just write down which one you're adding here so that we don't conflict. ;)

I just file a new PR to change the way we declare the lint rules. #96805
This can eliminate repeatedly rules declaration if we need to add further lint rules.

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue May 8, 2022
…iddle

Add more eslint rules

Slowly continuing to enforce more rules with eslint.

Part of rust-lang#93058.

r? `@notriddle`
@GuillaumeGomez
Copy link
Member

This issue is done so closing it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-rustdoc-js Area: Rustdoc's JS front-end T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

4 participants