-
-
Notifications
You must be signed in to change notification settings - Fork 274
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[code-infra] Closer sync with eslint config of codebase #3441
Changes from 1 commit
5af78f9
7297bf2
6f31a85
16adf62
3c127d9
1709cd7
cc9ce85
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4,6 +4,31 @@ const lodash = require('lodash'); | |
|
||
const ALLOWED_LODASH_METHODS = new Set(['throttle', 'debounce', 'set']); | ||
|
||
const noRestrictedImports = { | ||
paths: [ | ||
{ | ||
name: '@mui/icons-material', | ||
message: 'Use @mui/icons-material/<Icon> instead.', | ||
}, | ||
{ | ||
name: 'lodash-es', | ||
importNames: Object.keys(lodash).filter((key) => !ALLOWED_LODASH_METHODS.has(key)), | ||
message: | ||
'Avoid kitchensink libraries like lodash-es. We prefer a slightly more verbose, but more universally understood javascript style', | ||
}, | ||
{ | ||
name: 'react-query', | ||
message: 'deprecated package, use @tanstack/react-query instead.', | ||
}, | ||
], | ||
patterns: [ | ||
{ | ||
group: ['lodash-es/*'], | ||
message: 'Use `import { debounce } from "lodash-es";` instead.', | ||
}, | ||
], | ||
}; | ||
|
||
module.exports = { | ||
...baseline, | ||
settings: { | ||
|
@@ -25,37 +50,11 @@ module.exports = { | |
*/ | ||
rules: { | ||
...baseline.rules, | ||
'import/prefer-default-export': ['off'], | ||
// TODO move to @mui/monorepo, codebase is moving away from default exports | ||
'import/prefer-default-export': 'off', | ||
// TODO move rule into the main repo once it has upgraded | ||
'@typescript-eslint/return-await': ['off'], | ||
|
||
'no-restricted-imports': [ | ||
'error', | ||
{ | ||
paths: [ | ||
{ | ||
name: '@mui/icons-material', | ||
message: 'Use @mui/icons-material/<Icon> instead.', | ||
}, | ||
{ | ||
name: 'lodash-es', | ||
importNames: Object.keys(lodash).filter((key) => !ALLOWED_LODASH_METHODS.has(key)), | ||
message: | ||
'Avoid kitchensink libraries like lodash-es. We prefer a slightly more verbose, but more universally understood javascript style', | ||
}, | ||
{ | ||
name: 'react-query', | ||
message: 'deprecated package, use @tanstack/react-query instead.', | ||
}, | ||
], | ||
patterns: [ | ||
{ | ||
group: ['lodash-es/*'], | ||
message: 'Use `import { debounce } from "lodash-es";` instead.', | ||
}, | ||
], | ||
}, | ||
], | ||
'@typescript-eslint/return-await': 'off', | ||
'no-restricted-imports': ['error', noRestrictedImports], | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. moved to a variable, |
||
'no-restricted-syntax': [ | ||
...baseline.rules['no-restricted-syntax'].filter((rule) => { | ||
// Too opinionated for Toolpad | ||
|
@@ -94,15 +93,21 @@ module.exports = { | |
], | ||
}, | ||
], | ||
'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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The main difference |
||
{ | ||
files: ['docs/src/modules/components/**/*.js'], | ||
rules: { | ||
'material-ui/no-hardcoded-labels': 'off', | ||
}, | ||
}, | ||
{ | ||
files: ['examples/**/*'], | ||
rules: { | ||
// We use it for demonstration purposes | ||
'no-console': 'off', | ||
// Personal preference | ||
'no-underscore-dangle': 'off', | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Seems marginally used There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There is a reason. examples don't contain eslint. So the code shouldn't contain eslint comments. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 👍 I'm good with that as well |
||
// no node_modules in examples as they are not installed | ||
'import/no-unresolved': 'off', | ||
}, | ||
|
@@ -158,13 +163,11 @@ module.exports = { | |
'react/function-component-definition': 'off', | ||
}, | ||
}, | ||
// TODO remove, fix this rule in the codebase | ||
{ | ||
// Disabling this rule for now: | ||
// https://github.com/mui/material-ui/blob/9737bc85bb6960adb742e7709e9c3710c4b6cedd/.eslintrc.js#L359 | ||
files: ['packages/*/src/**/*{.ts,.tsx,.js}'], | ||
excludedFiles: ['*.d.ts', '*.spec.ts', '*.spec.tsx'], | ||
files: ['**'], | ||
rules: { | ||
'import/no-cycle': ['error', { ignoreExternal: true }], | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Removed seems to duplicate |
||
'no-restricted-imports': ['error', noRestrictedImports], | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Added until fixed in Toolpad |
||
}, | ||
}, | ||
], | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,8 @@ | ||
/* eslint-disable filenames/match-exported */ | ||
import * as React from 'react'; | ||
import { DatePicker } from '@toolpad/studio-components'; | ||
|
||
// TODO fix name | ||
export default function BasicDatepicker() { | ||
return <DatePicker label="Enter date" size="small" />; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,8 @@ | ||
/* eslint-disable filenames/match-exported */ | ||
import * as React from 'react'; | ||
import { List } from '@toolpad/studio-components'; | ||
|
||
// TODO fix name | ||
Janpot marked this conversation as resolved.
Show resolved
Hide resolved
|
||
export default function BasicList() { | ||
return <List itemCount={2} />; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,4 @@ | ||
/* eslint-disable no-underscore-dangle */ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. |
||
import { PrismaClient, Prisma } from '@prisma/client'; | ||
|
||
// Reuse existing PrismaClient instance during development | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Syntax convention (['off'] -> 'off')