Upgrade eslint #224
Upgrade eslint #224
Conversation
There was a problem hiding this comment.
Does this need to be .mjs? The ESLint docs has this just as .js
There was a problem hiding this comment.
ESLint docs also allows mjs and cjs extensions.
Used mjs here to allow modern "import/export" style rather than "require/module.exports", without changing our package.json to type:module (which may have other implications).
| ecmaFeatures: { jsx: true }, | ||
| }, | ||
| globals: { | ||
| /* replacement for env.node:true in eslintrc */ |
There was a problem hiding this comment.
Did we actually need this?
There was a problem hiding this comment.
yeah "env" is not supported anymore so we need to define it in globals
https://eslint.org/docs/latest/use/configure/migration-guide#configuring-language-options
in our case, mostly for "console" and "NodeJS.Timeout" type we use in few places.
|
|
||
| export class PerpExecuteAPI extends BaseVertexAPI { | ||
| async settlePnl(params: SettlePnlParams) { | ||
| async settlePnl(/*params: SettlePnlParams*/) { |
There was a problem hiding this comment.
nit: We could potentially remove the param for now
| import tsParser from '@typescript-eslint/parser'; | ||
| import tsPlugin from '@typescript-eslint/eslint-plugin'; | ||
| import reactPlugin from 'eslint-plugin-react'; | ||
| import reactHooksPlugin from 'eslint-plugin-react-hooks'; |
There was a problem hiding this comment.
Does this support eslint v9? facebook/react#28313 (comment)
There was a problem hiding this comment.
We could hold off on this until all dependencies support v9 - not a huge deal. We could potentially upgrade prettier though
There was a problem hiding this comment.
aaah, good catch, support look pretty close as it only fails when an error needs to be reported it seems.
using eslint-plugin-react@canary it reports errors normally if we want to go ahead without waiting a stable version release.
updated.
There was a problem hiding this comment.
I think we can probably park this until a stable release is out
| @@ -0,0 +1,54 @@ | |||
| import globals from "globals"; | |||
| import js from '@eslint/js'; | |||
| import tsParser from '@typescript-eslint/parser'; | |||
There was a problem hiding this comment.
Out of curiosity, typescript-eslint recommends using their own .config fn, would this be useful? https://typescript-eslint.io/getting-started
There was a problem hiding this comment.
doesn't feel especially handy as we also use other rules that do not support this "config fn" construct.
| }, | ||
| rules: { | ||
| ...js.configs.recommended.rules, | ||
| ...tsPlugin.configs['eslint-recommended'].rules, |
There was a problem hiding this comment.
Do you know the difference between this & .recommended below?
There was a problem hiding this comment.
It seems to be there to adjust core "js" eslint rules above to work better with typescript - eslint-recommended
This PR upgrades prettier and migrates to eslint 9.
The recommended set rules have changed slightly, so this PR also fixes newly triggered (minor) violations.
In our case, recommended rule set had some rules set to "warn" in earlier versions.instead of "error" now (no-unused-vars and no-explicit-any)