refactor: extend eslint #11934
refactor: extend eslint #11934
Conversation
TylerAPfledderer
requested review from
wackerow,
corwintines,
pettinarip,
minimalsm and
nhsz
as code owners
✅ Deploy Preview for ethereumorg ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
github-actions
bot
added
content 🖋️
|
Throwing back into draft to address type errors caught after running |
| @@ -1,11 +1,20 @@ | |||
| { | |||
| "extends": [ | |||
| "eslint:recommended", | |||
There was a problem hiding this comment.
Updated this configuration. Each line does the following:
eslint:recommended: follows the recommended rules as defined in the eslint docs with the ✅plugin:@typescript-eslint/recommended: follows the recommended rules as defined in this rule config and defined in the general rules tablenext/core-web-vitals: includes NextJS base lint config plus two rules as defined within this rules configprettier: have prettier rules override conflicting eslint rulesplugin:storybook/recommended: lint rules specific to storybook files
|
|
||
| import type { Lang } from "../lib/types" | ||
|
|
||
| type Summary = Record<string, string[]> | ||
|
|
||
| const argv = require("minimist")(process.argv.slice(2)) | ||
| const argv = minimist(process.argv.slice(2)) |
There was a problem hiding this comment.
If the require statement was left, there would be the following error:
Error: Require statement not part of import statement. @typescript-eslint/no-var-requires
There was a problem hiding this comment.
Looking good overall!
cc @pettinarip if you have thoughts, but this looks almost ready
| "simple-import-sort/exports": "error" | ||
| "simple-import-sort/exports": "error", | ||
| "no-unused-vars": "off", | ||
| "@typescript-eslint/no-unused-vars": "off", |
There was a problem hiding this comment.
Can we just modify to use "^_$" so the _ must be the first/only/last character for it to match? Then we can make that an error, without it being triggered by the _ placeholder variables
| "simple-import-sort/exports": "error" | ||
| "simple-import-sort/exports": "error", | ||
| "no-unused-vars": "off", | ||
| "@typescript-eslint/no-unused-vars": "off", |
There was a problem hiding this comment.
I don't think we need to not allow an arg like _variable, just want to make sure we can still use _ as an unused placeholder var. Sounds like this is good where it's at.
Originally in the NextJS forked repo as ethereum#267
Description
This PR introduces the following packages for the Eslint configuration:
@typescript-eslint/eslint-plugin: extends a recommended set of rules for Typescript and supersedes some existing eslint rules for improved checks.@typescript-eslint/parser: required parser for all sourced fileseslint-plugin-unused-imports: plugin to allow for auto fixing unused importsSee typescript-eslint docs to view the recommended rules configuration and other possible configurations available.
See the eslint-plugin-unused-imports repo for an overview of this plugin.
In addition, add the
nextconfiguration along withnext/core-web-vitalsto ensure the framework's recommended rules are being accounted for.Additional Information
In the eslint config, extending
@typescript-eslint:recommended-type-checkedinstead of*:recommendedcatches unsafe types, unsafe type overrides, unnecessary type assertions, etc. that can bleed into the project. i.e.JSON.parse()will always returnany, and adding a type assertion to it would be beneficial if using the resulting object later.Related Issue