[code-infra] Closer sync with eslint config of codebase#3441
Conversation
oliviertassinari
added
the
scope: code-infra
| excludedFiles: ['*.d.ts', '*.spec.ts', '*.spec.tsx'], | ||
| files: ['**'], | ||
| rules: { | ||
| 'import/no-cycle': ['error', { ignoreExternal: true }], |
There was a problem hiding this comment.
Removed seems to duplicate
| files: ['**'], | ||
| rules: { | ||
| 'import/no-cycle': ['error', { ignoreExternal: true }], | ||
| 'no-restricted-imports': ['error', noRestrictedImports], |
There was a problem hiding this comment.
Added until fixed in Toolpad
| // Personal preference | ||
| 'no-underscore-dangle': 'off', |
There was a problem hiding this comment.
Seems marginally used
There was a problem hiding this comment.
There is a reason. examples don't contain eslint. So the code shouldn't contain eslint comments.
There was a problem hiding this comment.
This makes sense. Should we move this rule to the share eslint config then? It feels like we want this for all /examples folders through MUI codebase.
| 'import/prefer-default-export': ['off'], | ||
| // TODO move to @mui/monorepo, codebase is moving away from default exports | ||
| 'import/prefer-default-export': 'off', |
There was a problem hiding this comment.
Syntax convention (['off'] -> 'off')
| }, | ||
| ], | ||
| '@typescript-eslint/return-await': 'off', | ||
| 'no-restricted-imports': ['error', noRestrictedImports], |
There was a problem hiding this comment.
moved to a variable, noRestrictedImports, to be able to extend it in specific paths.
| 'material-ui/no-hardcoded-labels': 'off', // We are not really translating the docs/website anymore | ||
| }, | ||
| overrides: [ | ||
| ...baseline.overrides, |
There was a problem hiding this comment.
The main difference
| @@ -1,3 +1,4 @@ | |||
| /* eslint-disable no-underscore-dangle */ | |||
There was a problem hiding this comment.
In general we don't add eslint disable comments in examples, but turn off the rule in an eslint override.
Reason is that eslint is not used/installed in the example, so it shouldn't contain eslint comments after I download it.
|
Did you see that eslint is in the process of deprecating its config format? The new format seems considerably easier to understand in terms of extending lint configs. |
I briefly looked at it, but saw we are on ESLint v8 not v9 so I ignored it. I have pushed in the past to not add any of those: https://github.com/mui/material-ui/blob/next/packages/api-docs-builder/.eslintrc.js. I'm happy it won't be possible anymore. If we want to prepare the migration, we could remove more of those. |
Janpot
left a comment
Janpot
left a comment
There was a problem hiding this comment.
Fixed the naming and brought back the rule for examples. With clearer comment. Want to merge this as I see PRs coming in that would fail the new naming rules.
2 participants
A step to simplify the eslint config in the codebase.