Skip to content
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.

Sign up for GitHub

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

ENH Loosen rules #13

Merged
merged 1 commit into from
Jan 10, 2023
Merged

ENH Loosen rules #13

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
171 changes: 163 additions & 8 deletions .eslintrc.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,101 @@ const todo = {
// can use refs instead
'off',
],
'react/default-props-match-prop-types': [
'off'
],
'react/no-access-state-in-setstate': [
'off'
],
'default-param-last': [
'off'
],
'max-classes-per-file': [
'off'
],
'no-param-reassign': [
'off'
],
'no-unused-vars': [
'off',
{
'vars': 'local',
'ignoreRestSiblings': true
}
],
'implicit-arrow-linebreak': [
'off'
],
'no-param-reassign': [
'off'
],
'no-redeclare': [
'off'
],
'no-restricted-globals': [
'off'
],
'no-dupe-keys': [
'off'
],
// the following can be automatically fixed via the --fix option
'import/order': [
'off'
],
'function-paren-newline': [
'off'
],
'object-curly-newline': [
'off'
],
'no-multiple-empty-lines': [
'off'
],
'prefer-object-spread': [
'off'
],
'operator-linebreak': [
'off'
],
'no-else-return': [
'off'
],
'function-call-argument-newline': [
'off'
],
'no-unneeded-ternary': [
'off'
],
'semi-style': [
'off'
],
'lines-between-class-members': [
'off'
],
'react/jsx-curly-newline': [
'off'
],
'react/jsx-wrap-multilines': [
'off'
],
'react/jsx-tag-spacing': [
'off'
],
'react/jsx-one-expression-per-line': [
'off'
],
'react/jsx-fragments': [
'off'
],
'react/jsx-curly-brace-presence': [
'off'
],
'react/jsx-closing-tag-location': [
'off'
],
'react/forbid-foreign-prop-types': [
'off'
]
};

module.exports = {
Expand All @@ -41,11 +136,20 @@ module.exports = {
'rules': Object.assign({},
todo,
{
"max-len": [
"error",
{
"code": 200,
Copy link
Member Author

@emteknetnz emteknetnz Jan 10, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

200 character line limit is required to support things like this - https://github.com/silverstripe/silverstripe-admin/blob/1/client/src/lib/dependency-injection/tests/ApolloGraphqlScaffoldingContainer-test.js#L164

Presumably there previously wasn't a line limit

Copy link
Member

Choose a reason for hiding this comment

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

Arguably things like that shouldn't exist, and should be refactored into something we can actually read and interpret. Something for us to consider when revisiting these rules.

}
],
// turned off otherwise non-admin modules will complain about importing components from admin
// via the novel silverstripe js component sharing setup
'import/no-extraneous-dependencies': [
Copy link
Member Author

@emteknetnz emteknetnz Jan 10, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

import/no-extraneous-dependencies is no longer the warning being thrown in most cases - but not all caseds, so need to change it to import/no-unresolved

All of the inline // eslint-disable-next-line import/no-extraneous-dependencies scattered out through the codebase probably were not doing much before, so I've removed them all

Copy link
Member

@GuySartorelli GuySartorelli Jan 10, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

so I've removed them all

As in there are separate PRs for this in silverstripe/admin, etc? Where are those PRs? Or did I misunderstand this statement?
Edit: nvm I see your comment further down saying we shouldn't touch the other modules for now. Agreed.

'off'
],
'import/no-unresolved': [
'off'
],
// turned off because the PHP side returns dangling properties which trigger this...
// could revise later and add exceptions for PHP data
'no-underscore-dangle': [
Expand All @@ -57,13 +161,6 @@ module.exports = {
'allowAfterThis': true
}
],
'no-unused-vars': [
'error',
{
'vars': 'local',
'ignoreRestSiblings': true
}
],
// increased to error because it's strongly discouraged
'react/no-danger': [
'error'
Expand All @@ -83,7 +180,7 @@ module.exports = {
'off'
],
'react/prefer-stateless-function': [
'error',
Copy link
Member Author

Choose a reason for hiding this comment

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

This didn't used to throw errors, possibly the detection is now better

'off',
{ 'ignorePureComponents': true }
],
'import/prefer-default-export': [
Expand All @@ -110,9 +207,67 @@ module.exports = {
'jsx-a11y/anchor-has-content': [
'off'
],
'jsx-a11y/control-has-associated-label': [
'off'
],
'jsx-a11y/click-events-have-key-events': [
'off'
],
'jsx-a11y/no-static-element-interactions': [
'off'
],
'jsx-a11y/anchor-is-valid': [
'off'
],
'no-prototype-builtins': [
'off'
],
'prefer-destructuring': [
'off'
],
'prefer-promise-reject-errors': [
'off'
],
'no-promise-executor-return': [
'off'
],
'func-names': [
'off'
],
'react/destructuring-assignment': [
'off'
],
'react/jsx-props-no-spreading': [
'off'
],
'react/button-has-type': [
'off'
],
'react/no-array-index-key': [
'off'
],
'react/jsx-indent': [
'off'
],
'react/sort-comp': [
'off'
],
'react/no-unused-prop-types': [
'off'
],
'react/no-unused-class-component-methods': [
'off'
],
'react/function-component-definition': [
'off'
],
// raise warnings because it may cause weird bugs
'react/jsx-no-constructed-context-values': [
'warn'
],
'react/no-unstable-nested-components': [
'warn'
],
}),
'overrides': [
{
Expand Down