Skip to content
Open
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
93 changes: 93 additions & 0 deletions packages/eslint-config-custom/react.js

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suggest the following rules to be added

"no-unused-vars": 2,
"no-var": 2,
"prefer-const": 2,
"no-eval": 2,
"no-magic-numbers": 2,

Copy link
Collaborator Author

@jinmayamashita jinmayamashita May 13, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@seiya0914

Thank you for the proposal! I've reviewed rules and it looks good, so I've added it.

ea971a3

It would be helpful if you could discuss with @mvn-luanvu-hn whether to approve the PR after reviewing the final rules.

Original file line number Diff line number Diff line change
@@ -0,0 +1,93 @@
module.exports = {
root: true,
env: {
browser: true,
es2021: true,
node: true,
},
settings: {
react: {
version: 'detect',
},
},
ignorePatterns: ['dist', '.eslintrc.cjs', 'prettier.config.cjs'],
extends: [
'eslint:recommended',
'plugin:@typescript-eslint/recommended',
'plugin:react/recommended',
'plugin:react/jsx-runtime',
'plugin:react-hooks/recommended',
],
plugins: ["unicorn"],
parser: '@typescript-eslint/parser',
parserOptions: {
project: ['./tsconfig.json', './tsconfig.node.json'],
},
rules: {
'no-console': 2,
eqeqeq: 2,
"no-unused-vars": 2,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we should use only single quotes or double quotes.

Copy link
Collaborator Author

@jinmayamashita jinmayamashita May 23, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree with you.

@mvn-luanvu-hn @seiya0914
What’s the difference between double and single quotes?
Can your team decide on the best practice?

"no-var": 2,
"prefer-const": 2,
"no-eval": 2,
"no-magic-numbers": 2,
"unicorn/filename-case": [2, { case: "kebabCase" }],
"react/jsx-filename-extension": [2, { extensions: [".tsx"] }],
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should add the following rule to disable when not import React in the component file

'react/react-in-jsx-scope': "off",

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In addition, we should add React Compiler's ESLint plugin to prepare our code to adapt to React Compiler in the future
https://react.dev/learn/react-compiler#installing-eslint-plugin-react-compiler

Copy link
Collaborator Author

@jinmayamashita jinmayamashita May 23, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should add the following rule to disable when not import React in the component file

We can add this plugin. I'll check it.

https://github.com/jsx-eslint/eslint-plugin-react/blob/577cb64fb6/README.md#configuration

If you are using the new JSX transform from React 17, extend react/jsx-runtime in your eslint config (add "plugin:react/jsx-runtime" to "extends") to disable the relevant rules.

// Best practices for writing efficient JavaScript code
"unicorn/no-array-for-each": 1,
"unicorn/no-array-push-push": 1,
"unicorn/no-array-reduce": 1,
"unicorn/no-for-loop": 1,
"unicorn/prefer-array-find": 1,
"unicorn/prefer-array-flat": 1,
"unicorn/prefer-array-flat-map": 1,
"unicorn/prefer-object-from-entries": 1,
"unicorn/no-useless-length-check": 1,
"unicorn/better-regex": 1,
"unicorn/no-negated-condition": 1,
"unicorn/prefer-add-event-listener": 1,
"unicorn/no-nested-ternary": 2,
// Code readability
"unicorn/prevent-abbreviations": [2, { allowList: { Props: true } }],
},
overrides: [
{
files: ['**/*.{jsx,ts,tsx}'],
rules: {
'react/jsx-pascal-case': 2,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wrote it in the discussion page, but I would like to discuss whether setting the severity to 2 (error) is appropriate.

It may be beneficial, especially for beginners, to enforce best practice, although it may become an annoyance to more experienced developers. Of course, it will vary on the rule, so I'd like to discuss with this in mind.

Regarding naming conventions, it may be enough to set it to 1 (warning).

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ptrkdan Normal syntax errors such as quotes and semicolons do not have a big impact on the normal working flow. I think these errors should be in the form of warning. For errors that affect the entire team's work flow, such as file name and export method, I think we should set level to error.

Copy link
Collaborator Author

@jinmayamashita jinmayamashita Mar 17, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ptrkdan The main reason for setting rule error is to prevent human oversight during code reviews. (Most of rule error is related to team development guidelines. If the rules are defined to a reasonable extent without being overwhelming, it might eliminate the need to explain them, and violations can be caught by CI failures, relieving reviewers from having to worry about them.)

If there are rules that may become noise for experienced developers, I believe it's possible to discuss and adjust them to rule warning.

'@typescript-eslint/naming-convention': [
2,
{
selector: 'variable',
format: ['camelCase', 'UPPER_CASE'],
},
{
selector: 'variable',
types: ['boolean'],
format: ['PascalCase'],
prefix: ['is', 'has', 'did', 'will', 'can', 'should'],
},
{
selector: ['variable', 'function'],
types: ['function'],
format: ['camelCase', 'StrictPascalCase'],
},
{
selector: ['interface', 'typeLike'],
format: ['PascalCase'],
},
],
// React components
'react/jsx-handler-names': 2,
'react/no-danger': 2,
'react/destructuring-assignment': [2, 'always'],
'react/hook-use-state': [2, { allowDestructuredState: false }],
'react/jsx-props-no-spreading': 2,
'react/no-unstable-nested-components': 2,
'react/prefer-stateless-function': 2,
'react/jsx-no-useless-fragment': 2,
'react/forbid-component-props': 2
},
},
],
};