ESLint - The Essential Linter and Formatter for JavaScript and TypeScript #1224

Greenlamp2 · 2024-05-22T00:29:12Z

Integrating ESLint for Improved Code Quality and Consistency

Introduction

ESLint is a powerful tool for identifying and fixing problems in JavaScript and TypeScript code. This addition will help us maintain a consistent code style, improve code quality, and catch potential bugs early.

Key Features

Automation:
- A pre-commit hook has been added to automatically run ESLint on the added or modified files, ensuring code quality before commits.
Manual Usage:
- If you prefer not to use the pre-commit hook, you can manually run ESLint to automatically fix issues using the command:
```
npx eslint --fix . or npm run eslint
```
- Running this command will lint all files in the repository.
GitHub Action:
- A GitHub Action has been added to automatically run ESLint on every push and pull request, ensuring code quality in the CI/CD pipeline.

Summary of ESLint Rules

General Rules:
- Equality: Use === and !== instead of == and != (eqeqeq).
- Indentation: Enforce 2-space indentation (indent).
- Quotes: Use doublequotes for strings (quotes).
- Variable Declarations:
  - Disallow var; use let or const (no-var).
  - Prefer const for variables that are never reassigned (prefer-const).
- Unused Variables: Allow unused function parameters but enforce error for other unused variables (@typescript-eslint/no-unused-vars).
- End of Line: Ensure at least one newline at the end of files (eol-last).
- Curly Braces: Enforce the use of curly braces for all control statements (curly).
- Brace Style: Use one true brace style (1tbs) for TypeScript-specific syntax (@typescript-eslint/brace-style).
TypeScript-Specific Rules:
- Semicolons:
  - Enforce semicolons for TypeScript-specific syntax (@typescript-eslint/semi).
  - Disallow unnecessary semicolons (@typescript-eslint/no-extra-semi).

Benefits

Consistency: Ensures consistent coding style across the project.
Code Quality: Helps catch potential errors and improve overall code quality.
Readability: Makes the codebase easier to read and maintain.

Conclusion

Integrating ESLint with these rules will greatly benefit our project by maintaining a high standard of code quality and consistency. Please review the PR and provide your feedback.

Thank you for your attention and cooperation!

Feel free to reach out if you have any questions or need further clarification on the ESLint rules. Happy coding!

_{^{Note: This presentation was assisted by AI.}}

This reverts commit 77a88e0.

…mmit (if wanted)

OrangeRed · 2024-05-22T01:23:29Z

Wasn't the issue with eslint and an auto formatter that a different dev didn't like the output of the formatted code?

Also secretly hoping we enable strictNullChecks in the tsconfig

bennybroseph · 2024-05-22T05:57:03Z

Wasn't the issue with eslint and an auto formatter that a different dev didn't like the output of the formatted code?

Also secretly hoping we enable strictNullChecks in the tsconfig

Well, it's customizable so we are discussing the rules internally at the moment.

francktrouillez · 2024-05-22T06:49:52Z

@Greenlamp2
Thank you very much for that! Would be so much more comfortable working in this project once this is setup!

Wasn't the issue with eslint and an auto formatter that a different dev didn't like the output of the formatted code?

IMHO, I very much prefer to have a standardized way of doing things than to let everyone bring their own convention. For instance, I usually prefer double-quote strings, but I very much prefer that I'm required to use single-quote strings rather than having both existing in the same project.

francktrouillez

@Greenlamp2
Thanks for this initiative!

If you prefer not to use the pre-commit hook, you can manually run ESLint to automatically fix issues using the command

Do we want to allow that? In the sense that I believe we should enforce the rules in the repository, as to avoid inconsistencies in the codebase.

What do you think? cc: @bennybroseph
Also, on another note, do you think it would make sense to enable a Github Action (like this one) to run the linter in PRs as well, just to avoid inconsistencies in the codebase if someone bypasses the hooks (either by not installing the hooks in the first place, or by committing with --no-verify)?
I know the free-tier includes 2000 minutes per month, but I honestly have no idea about the time the linter usually takes to run on a file, and how many times the action will be run. I believe before diving into that, it would be great to know if that could be an enhancement of the current process or not.

What do you think?

README.md

Greenlamp2 · 2024-05-22T09:18:37Z

@Greenlamp2 Thanks for this initiative!

If you prefer not to use the pre-commit hook, you can manually run ESLint to automatically fix issues using the command

Do we want to allow that? In the sense that I believe we should enforce the rules in the repository, as to avoid inconsistencies in the codebase.
What do you think? cc: @bennybroseph

Also, on another note, do you think it would make sense to enable a Github Action (like this one) to run the linter in PRs as well, just to avoid inconsistencies in the codebase if someone bypasses the hooks (either by not installing the hooks in the first place, or by committing with --no-verify)?
I know the free-tier includes 2000 minutes per month, but I honestly have no idea about the time the linter usually takes to run on a file, and how many times the action will be run. I believe before diving into that, it would be great to know if that could be an enhancement of the current process or not.
What do you think?

yes a Github action would be perfect, we can try it, 2000min can be translated to around 4000 commits, i don't know how many commits we do every month though, it will need to be tracked somehow.

…eslint # Conflicts: # src/battle-scene.ts # src/battle.ts # src/data/ability.ts # src/field/pokemon.ts # src/game-mode.ts # src/locales/de/voucher.ts # src/locales/es/battle-message-ui-handler.ts # src/locales/es/berry.ts # src/locales/es/menu-ui-handler.ts # src/locales/es/pokemon-info.ts # src/phases.ts # src/ui/text.ts # src/utils.ts

…at/eslint # Conflicts: # src/battle-scene.ts # src/battle.ts # src/data/ability.ts # src/field/pokemon.ts # src/game-mode.ts # src/locales/de/voucher.ts # src/locales/es/menu-ui-handler.ts # src/locales/es/pokemon-info.ts # src/phases.ts # src/ui/text.ts # src/utils.ts

Greenlamp2 · 2024-05-23T07:39:38Z

stage_fixed: true in the lefthook.yml is to add the linted files to the commit, otherwise a second commit with eslint would be needed

francktrouillez

Looks good to me! Thanks for taking the time to work on that!

I just left a comment about updating the README, but the configuration and the workflow looks great!

README.md

francktrouillez · 2024-05-23T09:50:47Z

@Greenlamp2
Also, that's a nit but do you think we could try to clean up the history by squashing some commits together? That's just personal preferences here, so, up-to-you!

Greenlamp2 · 2024-05-23T09:52:03Z

@Greenlamp2 Also, that's a nit but do you think we could try to clean up the history by squashing some commits together? That's just personal preferences here, so, up-to-you!

everything will be squashed on merge, don't worry

brain-frog

it begins

…ript (pagefaultgames#1224) * eslint config + packages * updated eslint config * fix the issue eslint adding ;;;; at interfaces * first round with eslint --fix . * removed config for unused export * Revert "first round with eslint --fix ." This reverts commit 77a88e0. * removed config for camelCase * for real this time, first round of eslint --fix . * halfway to manual eslint fix * eslint done * added "how to setup" the hook to eslint --fix each new file before commit (if wanted) * removed eslintrc config file duplicat * fix human error + ignore build folder + merge overrides * added curly brace style + eslint * applied double quote linter rule * added lefthook * test precommit * test precommit * test precommit * test precommit * test precommit * test precommit * test precommit * github action to run eslint * added node_modules to ignore eslint * different action for typescript * no need for different glob (default src) * node 20 * node 20 * removed no longer needed install file * remove hooks part from README * eslint fixes --------- Co-authored-by: Frederico Santos <frederico.f.santos@tecnico.ulisboa.pt>

Greenlamp2 added 12 commits May 22, 2024 00:32

eslint config + packages

616f011

updated eslint config

da9b59c

fix the issue eslint adding ;;;; at interfaces

8eb6f8a

first round with eslint --fix .

77a88e0

removed config for unused export

7633b4e

Revert "first round with eslint --fix ."

164f292

This reverts commit 77a88e0.

removed config for camelCase

dc82cdb

for real this time, first round of eslint --fix .

28ada38

halfway to manual eslint fix

8e25060

eslint done

9a8b604

added "how to setup" the hook to eslint --fix each new file before co…

78f8abc

…mmit (if wanted)

removed eslintrc config file duplicat

ebb7a81

Tempo-anon added the Enhancement New feature or request label May 22, 2024

francktrouillez reviewed May 22, 2024

View reviewed changes

README.md Outdated Show resolved Hide resolved

f-fsantos self-assigned this May 22, 2024

Greenlamp2 and others added 5 commits May 23, 2024 00:57

fix human error + ignore build folder + merge overrides

1672206

added curly brace style + eslint

b0371fc

Merge branch 'main-pokerogue' into feat/eslint

f7baca1

applied double quote linter rule

be1e15a

f-fsantos approved these changes May 23, 2024

View reviewed changes

Greenlamp2 added 5 commits May 23, 2024 09:08

added lefthook

75cb018

test precommit

b9eadd0

test precommit

94a5f0e

test precommit

ae6800d

Greenlamp2 added 4 commits May 23, 2024 09:28

test precommit

dee00be

test precommit

d376737

test precommit

cd6be50

test precommit

a6f1c3e

Greenlamp2 added 2 commits May 23, 2024 09:46

github action to run eslint

21e2551

added node_modules to ignore eslint

00d7301

Greenlamp2 marked this pull request as draft May 23, 2024 07:50

Greenlamp2 added 4 commits May 23, 2024 09:52

different action for typescript

5cd26a6

no need for different glob (default src)

c501e20

node 20

1c9cf41

node 20

ac41fb3

Greenlamp2 marked this pull request as ready for review May 23, 2024 08:05

removed no longer needed install file

a07e6b3

francktrouillez approved these changes May 23, 2024

View reviewed changes

README.md Outdated Show resolved Hide resolved

remove hooks part from README

54151c6

f-fsantos approved these changes May 23, 2024

View reviewed changes

f-fsantos added 2 commits May 23, 2024 14:28

Merge branch 'main-pokerogue' into feat/eslint

2e6f1fa

eslint fixes

eafae6b

brain-frog approved these changes May 23, 2024

View reviewed changes

brain-frog merged commit bac6c22 into pagefaultgames:main May 23, 2024
2 checks passed

Greenlamp2 deleted the feat/eslint branch May 27, 2024 10:07

flx-sta unassigned f-fsantos Sep 17, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ESLint - The Essential Linter and Formatter for JavaScript and TypeScript #1224

ESLint - The Essential Linter and Formatter for JavaScript and TypeScript #1224

Greenlamp2 commented May 22, 2024 •

edited

Loading

OrangeRed commented May 22, 2024

bennybroseph commented May 22, 2024

francktrouillez commented May 22, 2024

francktrouillez left a comment •

edited

Loading

Greenlamp2 commented May 22, 2024

Greenlamp2 commented May 23, 2024

francktrouillez left a comment

francktrouillez commented May 23, 2024

Greenlamp2 commented May 23, 2024

brain-frog left a comment

ESLint - The Essential Linter and Formatter for JavaScript and TypeScript #1224

ESLint - The Essential Linter and Formatter for JavaScript and TypeScript #1224

Conversation

Greenlamp2 commented May 22, 2024 • edited Loading

Integrating ESLint for Improved Code Quality and Consistency

Introduction

Key Features

Summary of ESLint Rules

Benefits

Conclusion

OrangeRed commented May 22, 2024

bennybroseph commented May 22, 2024

francktrouillez commented May 22, 2024

francktrouillez left a comment • edited Loading

Choose a reason for hiding this comment

Greenlamp2 commented May 22, 2024

Greenlamp2 commented May 23, 2024

francktrouillez left a comment

Choose a reason for hiding this comment

francktrouillez commented May 23, 2024

Greenlamp2 commented May 23, 2024

brain-frog left a comment

Choose a reason for hiding this comment

Greenlamp2 commented May 22, 2024 •

edited

Loading

francktrouillez left a comment •

edited

Loading