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

style: var -> let and const codemod #4444

Closed
wants to merge 9 commits into from
Closed

style: var -> let and const codemod #4444

wants to merge 9 commits into from

Conversation

gaiety-deque
Copy link
Contributor

adds no-var rule to eslint
(this also pairs well with the already enabled prefer-const and no-redeclare)

Who else does this?

Why do this?

Gaiety's explanation: "let works the way most devs think var works. scope leaking and variable hoisting is a messy confusing idea, JS got updated for a reason" also it's 2024, this isn't too controversial anymore

<< Describe the changes >>

Closes:

adds `no-var` rule to eslint

Who else does this?
  - Google (JS style guide) https://google.github.io/styleguide/jsguide.html#features-local-variable-declarations
  - Airbnb (popular JS style guide) https://github.com/airbnb/javascript

Why do this?

Gaiety's explanation: "`let` works the way most devs think `var` works.
scope leaking and variable hoisting is a messy confusing idea, JS got
updated for a reason" also it's 2024, this isn't too controversial
anymore

- https://www.freecodecamp.org/news/var-let-and-const-whats-the-difference/
- https://blog.javascripttoday.com/blog/var-vs-let-vs-const/ "The var
  keyword was originally used for declaring variables in JavaScript. If
  you’ve browsed an old codebase or an older JavaScript tutorial, you
  have probably seen it."
- https://medium.com/@rithinmenezes/understanding-let-var-and-const-in-javascript-a-guide-to-variable-declarations-f271c5b8dc79
- Airbnb's why https://airbnb.io/javascript/#variables
@@ -7,5 +7,6 @@ doc/api/*
doc/examples/jest_react/*.js

lib/core/imports/*.js
lib/core/utils/uuid.js
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not really our code, maybe belongs in a vendor file? Felt weird touching it.

@gaiety-deque gaiety-deque marked this pull request as ready for review May 3, 2024 18:48
@gaiety-deque gaiety-deque requested a review from a team as a code owner May 3, 2024 18:48
@gaiety-deque
Copy link
Contributor Author

Closing this in favor of doing smaller PR's piece by piece over time, but I'll reference the changes here as the ultimate goal.

gaiety-deque added a commit that referenced this pull request May 7, 2024
This is a piece of a larger creative time PR so it's easier to review,
the full scope can be seen here and includes eslint adjustments not seen
here #4444

---

Who else does this?
- Google (JS style guide)
https://google.github.io/styleguide/jsguide.html#features-local-variable-declarations
  - Airbnb (popular JS style guide) https://github.com/airbnb/javascript

Why do this?

Gaiety's explanation: "`let` works the way most devs think `var` works.
scope leaking and variable hoisting is a messy confusing idea, JS got
updated for a reason" also it's 2024, this isn't too controversial
anymore

-
https://www.freecodecamp.org/news/var-let-and-const-whats-the-difference/
- https://blog.javascripttoday.com/blog/var-vs-let-vs-const/ "The var
keyword was originally used for declaring variables in JavaScript. If
you’ve browsed an old codebase or an older JavaScript tutorial, you have
probably seen it."
-
https://medium.com/@rithinmenezes/understanding-let-var-and-const-in-javascript-a-guide-to-variable-declarations-f271c5b8dc79
- Airbnb's why https://airbnb.io/javascript/#variables
gaiety-deque added a commit that referenced this pull request May 9, 2024
gaiety-deque added a commit that referenced this pull request May 10, 2024
Just like #4451 but for
`lib/commons`

Smaller part of the full vision:
#4444
gaiety-deque added a commit that referenced this pull request Jun 6, 2024
Just like #4451 but for
`lib/core`

This does add an `.eslintignore` line, perhaps we'd rather skip a file a
different way.

Smaller part of the full vision:
#4444
gaiety-deque added a commit that referenced this pull request Jun 6, 2024
gaiety-deque added a commit that referenced this pull request Jun 7, 2024
gaiety-deque added a commit that referenced this pull request Jun 13, 2024
Just like #4451 but for
`lib/rules`. This is the last of `lib/*`

Smaller part of the full vision:
#4444

---------

Co-authored-by: Wilco Fiers <WilcoFiers@users.noreply.github.com>
WilcoFiers pushed a commit that referenced this pull request Jun 17, 2024
For `test/rule-matches`

Smaller part of the full vision:
#4444
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant