Add an ESLint rule to prevent the usage of useRef other than for HTML elements. #2014
Conversation
… elements Co-authored-by: v1b3m <vibenjamin6@gmail.com>
Apply Sweep Rules to your PR?
|
charlesBochet
left a comment
charlesBochet
left a comment
There was a problem hiding this comment.
That was quick! Left a comment :)
| @@ -0,0 +1,39 @@ | |||
| "use strict"; | |||
There was a problem hiding this comment.
we should not commit dist file.
Let's actually add dist folder to gitignore
There was a problem hiding this comment.
@charlesBochet dist in eslint-plugin-twenty is ignored, but eslint-plugin-twenty-ts was added and I figured, perhaps it serves a special purpose, hence the commits and updates
| 'plugin:prettier/recommended', | ||
| 'plugin:storybook/recommended', | ||
| 'react-app', | ||
| 'plugin:react/recommended', |
There was a problem hiding this comment.
I'm moving to these plugins as the react-app config is not compatible with the newer eslint version
| { | ||
| files: ['*.js', '*.jsx', '*.ts', '*.tsx'], | ||
| rules: { | ||
| 'react/no-unescaped-entities': 'off', |
There was a problem hiding this comment.
disabling these as the new plugin is stricter. I think it would be great to reactivate jsx-key one, others do not seem that useful (I'm not sure about prop-types)
| "chromatic": "dotenv cross-var npx chromatic --project-token=$CHROMATIC_PROJECT_TOKEN", | ||
| "install": "yarn eslint-plugin:setup" | ||
| }, | ||
| "eslintConfig": { |
There was a problem hiding this comment.
removing this section as we use .eslintrc
|
|
||
| const noStateUseRef = createRule({ | ||
| create: (context) => { | ||
| return { |
There was a problem hiding this comment.
@gitstart-twenty we prefer this way of writing if condition that avoids making a complex conditions and enforce runtime typing properly
| ], | ||
| }, | ||
| { | ||
| code: "const ref = useRef<string>('');", |
There was a problem hiding this comment.
added this case that was not taken into account
| return; | ||
| } | ||
|
|
||
| if (!node.typeArguments || !node.typeArguments.params?.length) { |
There was a problem hiding this comment.
using typeArguments instead of typeParameters which is deprecated :)
There was a problem hiding this comment.
@charlesBochet I saw this and made the necessary changes but then reverted them after I noticed the rule wasn't behaving as expected. This is because the AST generated by @typescript-eslint/parser uses typeParameters and not typeArguments
|
@gitstart-twenty I've left a few comments, please have a look for next times :) |
Thanks for the update |
Fixes #1996