The Materia project (and all other projects eventually) should be auto formatted and linted to prevent common mistakes and to increase readability. For this use I propose using Prettier and ESLint. For the uninformed, Prettier is an opinionated code formatter. ESLint is just a linter.
Anyone with any opinions on code style or formatting should read through these proposed changes, or the config in the eventual PR, and state their opinions before it is too late.
Prettier
Prettier requires specific formatting rules. The default prettier config is pretty good, but there are some changes that we would want. Such as no semicolons (you're welcome, @clpetersonucf), tabs instead of spaces, and forcing single quotes when possible. I do want to mention that this config was already in the package.json file, but it was unused.
{
"printWidth": 100,
"semi": false,
"useTabs": true,
"singleQuote": true
}
ESLint
ESlint is a bit more complicated. I think we need to at least use ESLint JS Recommended (these are all of the rules with the green checkmark on the ESLint Rules Page) and ESLint React Recommended. But I also compiled a list of additions that I personally like.
From Obojobo
These were from the Obojobo ESLint Config. Many of these are already included in the ESLint JS recommended config. Some of them are deprecated too. I included this here as reference.
| Rule Name |
Description |
array-callback-return |
Enforces return statements in array method callbacks. |
brace-style ❌ |
Enforces the "one true brace style" while allowing single-line blocks. |
curly |
Requires curly braces for all multi-line control statements. |
eqeqeq |
Requires the use of === and !==. |
new-cap |
Requires constructors to start with a capital letter (with specific exceptions). |
no-alert |
Disallows the use of alert, confirm, and prompt. |
no-console |
Disallows the use of console methods. |
no-debugger ✅ |
Disallows the use of debugger statements. |
no-duplicate-imports |
Disallows multiple imports from the same module. |
no-eval |
Disallows the use of the eval() function. |
no-extend-native |
Disallows extending native object prototypes. |
no-floating-decimal ❌ |
Disallows leading or trailing decimal points in numeric literals. |
no-implied-eval |
Disallows eval()-like methods (e.g., setTimeout with strings). |
no-labels |
Disallows labeled statements. |
no-lonely-if |
Disallows if statements as the only statement in else blocks. |
no-loop-func |
Disallows function declarations inside loops. |
no-nested-ternary |
Disallows nesting ternary expressions. |
no-new |
Disallows new operators outside of assignments or comparisons. |
no-new-func |
Disallows the Function constructor. |
no-new-wrappers |
Disallows creating new instances of String, Number, or Boolean. |
no-return-assign |
Disallows assignment operators in return statements. |
no-self-compare |
Disallows comparing a variable against itself. |
no-undefined |
Disallows the use of undefined as an identifier. |
no-unneeded-ternary |
Disallows ternary operators when simpler alternatives exist. |
no-unused-vars ✅ |
Disallows unused variables (ignoring specific patterns like next). |
no-useless-return |
Disallows redundant return statements. |
no-var |
Requires let or const instead of var. |
no-with ✅ |
Disallows the use of the with statement. |
prefer-const |
Requires const for variables that are never reassigned. |
radix |
Enforces the use of a radix argument in parseInt(). |
react/prop-types |
Disables the requirement for React prop-types validation. |
yoda |
Disallows "Yoda conditions" in comparisons. |
❌ - Deprecated
✅ - In recommended options
My Suggestions
- Error = do NOT do this
- Warn = we shouldn't do this, but someone can fix this later
I believe that warnings are generally to be avoided, unless code is being refactored or is in development. The reasoning being that warnings will just be ignored, and then they might as well not exist. They still should prevent merging into main, but they should not stop a push or a commit.
Error
Correctness
| Rule Name |
Description |
array-callback-return |
Ensures return statements are present in array callbacks. |
no-duplicate-imports |
Disallows multiple imports from the same module. |
no-self-compare |
Disallows comparing a variable against itself. |
no-template-curly-in-string |
Disallows template syntax inside regular strings. |
no-unreachable-loop |
Disallows loops that only allow one iteration. |
no-use-before-define |
Disallows using variables before they are defined. |
require-atomic-updates |
Prevents race conditions in assignments with await/yield. |
Suggestions
| Rule Name |
Description |
arrow-body-style |
Requires braces around arrow bodies as needed. |
camelcase |
Enforces camelCase naming conventions. |
capitalized-comments |
Enforces capitalization of the first letter of comments. |
class-methods-use-this |
Ensures class methods use 'this'. |
curly |
Enforces curly braces for all control statements. |
default-case |
Requires default cases in switch statements. |
default-case-last |
Enforces default clauses in switch are last. |
dot-notation |
Enforces dot notation over bracket notation. |
eqeqeq |
Requires === and !==. |
no-extend-native |
Disallows adding properties to native prototypes. |
no-extra-boolean-cast |
Disallows unnecessary boolean casts. |
no-implicit-coercion |
Disallows shorthand type conversions. |
no-implied-eval |
Disallows eval()-like methods. |
no-invalid-this |
Disallows 'this' outside of classes/objects. |
no-labels |
Disallows labeled statements. |
no-lonely-if |
Disallows if statements as the only statement in else blocks. |
no-magic-numbers |
Disallows unnamed numerical constants. |
no-multi-assign |
Disallows chained assignments. |
no-multi-str |
Disallows multiline strings via backslash. |
no-nested-ternary |
Disallows nesting ternary expressions. |
no-new-func |
Disallows the Function constructor. |
no-new-wrappers |
Disallows new instances of String, Number, or Boolean. |
no-object-constructor |
Disallows the Object constructor. |
no-param-reassign |
Disallows reassigning function parameters. |
no-return-assign |
Disallows assignment in return statements. |
no-script-url |
Disallows javascript: URLs. |
no-shadow |
Disallows variable shadowing. |
no-throw-literal |
Disallows throwing literals as exceptions. |
no-unneeded-ternary |
Disallows ternary operators for simple booleans. |
no-useless-constructor |
Disallows unnecessary constructors. |
no-useless-rename |
Disallows redundant renaming in imports/exports. |
no-useless-return |
Disallows redundant return statements. |
no-var |
Requires let or const instead of var. |
no-void |
Disallows the void operator. |
operator-assignment |
Requires assignment operator shorthand. |
prefer-arrow-callback |
Requires using arrow functions for callbacks. |
prefer-const |
Requires const for variables never reassigned. |
prefer-promise-reject-errors |
Requires Error objects in Promise rejection. |
prefer-template |
Requires template literals over concatenation. |
radix |
Enforces the radix argument for parseInt(). |
require-await |
Disallows async functions with no await. |
sort-imports |
Enforces sorted import declarations. |
yoda |
Disallows "Yoda conditions" in comparisons. |
Warn
Both of these are preferences that allow us to put in temp comments while developing, but need to be fixed before merged.
| Rule Name |
Description |
no-inline-comments |
Disallows comments on the same line as code. |
no-warning-comments |
Disallows warning terms like TODO or FIXME. |
Changes to ESLint/js recommended
A javascript standard is to allow unused variables given that they start with an underscore. This is often needed when multiple functions take the same positional parameters, but one function might not actually use them all.
no-unused-vars ["warn", { argsIgnorePattern: "^_" }]
CI and Pre-commit Hooks
There needs to be discussion on the processes for enforcement. At a minimum I would require checks for formatting and linting at the github workflow level, so that no changes can be merged if formatted improperly.
As it stands, Materia uses pre-commit for the backend pre-commit scripts. I recommend we add a repository-local hook to run lint-staged. We can have it setup to lint and format every file touched in a commit.
The Materia project (and all other projects eventually) should be auto formatted and linted to prevent common mistakes and to increase readability. For this use I propose using Prettier and ESLint. For the uninformed, Prettier is an opinionated code formatter. ESLint is just a linter.
Anyone with any opinions on code style or formatting should read through these proposed changes, or the config in the eventual PR, and state their opinions before it is too late.
Prettier
Prettier requires specific formatting rules. The default prettier config is pretty good, but there are some changes that we would want. Such as no semicolons (you're welcome, @clpetersonucf), tabs instead of spaces, and forcing single quotes when possible. I do want to mention that this config was already in the
package.jsonfile, but it was unused.{ "printWidth": 100, "semi": false, "useTabs": true, "singleQuote": true }ESLint
ESlint is a bit more complicated. I think we need to at least use ESLint JS Recommended (these are all of the rules with the green checkmark on the ESLint Rules Page) and ESLint React Recommended. But I also compiled a list of additions that I personally like.
From Obojobo
These were from the Obojobo ESLint Config. Many of these are already included in the ESLint JS recommended config. Some of them are deprecated too. I included this here as reference.
array-callback-returnbrace-style❌curlyeqeqeqnew-capno-alertno-consoleno-debugger✅no-duplicate-importsno-evalno-extend-nativeno-floating-decimal❌no-implied-evalno-labelsno-lonely-ifno-loop-funcno-nested-ternaryno-newno-new-funcno-new-wrappersno-return-assignno-self-compareno-undefinedno-unneeded-ternaryno-unused-vars✅no-useless-returnno-varno-with✅prefer-constradixreact/prop-typesyoda❌ - Deprecated
✅ - In recommended options
My Suggestions
I believe that warnings are generally to be avoided, unless code is being refactored or is in development. The reasoning being that warnings will just be ignored, and then they might as well not exist. They still should prevent merging into main, but they should not stop a push or a commit.
Error
Correctness
array-callback-returnno-duplicate-importsno-self-compareno-template-curly-in-stringno-unreachable-loopno-use-before-definerequire-atomic-updatesSuggestions
arrow-body-stylecamelcasecapitalized-commentsclass-methods-use-thiscurlydefault-casedefault-case-lastdot-notationeqeqeqno-extend-nativeno-extra-boolean-castno-implicit-coercionno-implied-evalno-invalid-thisno-labelsno-lonely-ifno-magic-numbersno-multi-assignno-multi-strno-nested-ternaryno-new-funcno-new-wrappersno-object-constructorno-param-reassignno-return-assignno-script-urlno-shadowno-throw-literalno-unneeded-ternaryno-useless-constructorno-useless-renameno-useless-returnno-varno-voidoperator-assignmentprefer-arrow-callbackprefer-constprefer-promise-reject-errorsprefer-templateradixrequire-awaitsort-importsyodaWarn
Both of these are preferences that allow us to put in temp comments while developing, but need to be fixed before merged.
no-inline-commentsno-warning-commentsChanges to ESLint/js recommended
A javascript standard is to allow unused variables given that they start with an underscore. This is often needed when multiple functions take the same positional parameters, but one function might not actually use them all.
no-unused-vars["warn", { argsIgnorePattern: "^_" }]CI and Pre-commit Hooks
There needs to be discussion on the processes for enforcement. At a minimum I would require checks for formatting and linting at the github workflow level, so that no changes can be merged if formatted improperly.
As it stands, Materia uses pre-commit for the backend pre-commit scripts. I recommend we add a repository-local hook to run lint-staged. We can have it setup to lint and format every file touched in a commit.