chore: update eslint to fix linting issues #29988
Conversation
gatsbot
bot
added
the
status: triage needed
| // if (process.env.NODE_ENV !== `test`) { | ||
| // ignore.push(`**/__tests__`) | ||
| // } |
There was a problem hiding this comment.
This breaks linting as babel will return an empty ast. Packages should be build with babel src --out-dir . --ignore \"**/__tests__\"
| "prettier/flowtype", | ||
| "prettier/react", |
There was a problem hiding this comment.
these are now inside prettier
| ], | ||
| plugins: ["flowtype", "prettier", "react", "filenames"], | ||
| plugins: [`flowtype`, `prettier`, `react`, `filenames`, `@babel`], |
There was a problem hiding this comment.
Added @babel to augment the this rules
| "prefer-promise-reject-errors": `warn`, | ||
| "no-prototype-builtins": `warn`, | ||
| "guard-for-in": `warn`, |
There was a problem hiding this comment.
These are new rules and moved them to warnings because there were too many.
.eslintrc.js
Outdated
| "prefer-promise-reject-errors": `warn`, | ||
| "no-prototype-builtins": `warn`, | ||
| "guard-for-in": `warn`, | ||
| "spaced-comment": [`error`, `always`, { markers: [`/`] }], |
There was a problem hiding this comment.
enable typescript /// referrence comments
| camelcase: [ | ||
| `error`, | ||
| { | ||
| properties: `never`, | ||
| ignoreDestructuring: true, | ||
| allow: [`^unstable_`], | ||
| }, | ||
| ], |
There was a problem hiding this comment.
I have no idea why this wasn't necessary before
| @@ -1,4 +1,4 @@ | |||
| /* global __PATH_PREFIX__ CMS_PUBLIC_PATH */ | |||
| /* global CMS_PUBLIC_PATH */ | |||
There was a problem hiding this comment.
PATH_PREFIX is in the global globals list
| if (node.url.indexOf(`images.ctfassets.net`) === -1) { | ||
| return resolve() | ||
| } |
There was a problem hiding this comment.
This is really weird and I'm astonished that this hasn't lead to any bugs.
| @@ -71,12 +71,12 @@ const doesConfigChangeRequireRestart = ( | |||
| const getDebugPort = (port?: number): number => port ?? 9229 | |||
|
|
|||
| export const getDebugInfo = (program: IProgram): IDebugInfo | null => { | |||
| if (program.hasOwnProperty(`inspect`)) { | |||
| if (Object.prototype.hasOwnProperty.call(program, `inspect`)) { | |||
There was a problem hiding this comment.
I was fixing this rule but eventually moved to warning.
There was a problem hiding this comment.
Can you explain why this is needed?
There was a problem hiding this comment.
https://eslint.org/docs/rules/no-prototype-builtins
It's a security risk as technically program could have overwritten hasOwnProperty
There was a problem hiding this comment.
Neat. I'd not thought of that
wardpeet
added
type: maintenance
| @@ -784,7 +784,7 @@ export const createWebpackUtils = ( | |||
| ], | |||
| ...eslintConfig(schema, jsxRuntimeExists), | |||
| } | |||
| //@ts-ignore | |||
| // @ts-ignore | |||
There was a problem hiding this comment.
Now I'm wondering why our ts rules are allowing this!
There was a problem hiding this comment.
Do we have a lint rule for the space in typescript?
There was a problem hiding this comment.
No, but we do have a rule that requires a description when there's a ts-ignore comment
There was a problem hiding this comment.
It's only a warning :)
"@typescript-eslint/ban-ts-comment": [
`warn`,
{ "ts-ignore": `allow-with-description` },
],
Co-authored-by: Matt Kane <matt@gatsbyjs.com>
Description
Upgraded eslint to latest and fixed rules along the way.
Documentation
Related Issues