Update eslint-plugins and drop eslint@6 support #210
Conversation
| "@typescript-eslint/eslint-plugin": "^4.1.0", | ||
| "@typescript-eslint/parser": "^4.1.0", | ||
| "@typescript-eslint/eslint-plugin": "^4.15.0", | ||
| "@typescript-eslint/parser": "^4.15.0", |
There was a problem hiding this comment.
Potential new rules for lib/config/rules/typescript.js:
object-curly-spacingsort-type-union-intersection-membersnon-nullable-type-assertion-styleno-confusing-void-expressionno-unnecessary-type-constraintspace-infix-opsconsistent-indexed-object-style
Also, naming-convention has a few new options in 4.9.0.
There was a problem hiding this comment.
I turned on the ones that work out of the box. There are a few that require the project option to be set on typescript-eslint/parser that I will need to figure out in another PR.
The sort-type-union-intersection-members will need more thought. It rejects a large amount of code, but it's not clear how to "fix" it.
There was a problem hiding this comment.
I wonder if we could code mod? Example on how to fix: https://github.com/typescript-eslint/typescript-eslint/blob/master/packages/eslint-plugin/docs/rules/sort-type-union-intersection-members.md That being said could this work be done in a follow PR?
vsumner
force-pushed
the
update-eslint-plugin-7
branch
from
62d4f3a
to
f7b0e41
Compare
| @@ -10,5 +10,6 @@ module.exports = { | |||
| rules: merge(require('./rules/node'), { | |||
| 'no-process-env': 'off', | |||
| 'no-console': 'off', | |||
| 'no-useless-token-range': 'off', | |||
There was a problem hiding this comment.
We are turning this off as stuff will break? This will make rules less performant...
https://github.com/not-an-aardvark/eslint-plugin-eslint-plugin/blob/master/docs/rules/no-useless-token-range.md
There was a problem hiding this comment.
I forgot I turned this off. Nice catch.
| @@ -10,6 +10,8 @@ module.exports = { | |||
| '@shopify/class-property-semi': 'off', | |||
| '@shopify/binary-assignment-parens': 'off', | |||
| 'babel/semi': 'off', | |||
| 'prefer-arrow-callback': 'off', | |||
There was a problem hiding this comment.
We are doing this to support function Foo(){} over const I assume? Or as things well break?
There was a problem hiding this comment.
The documentation recommended following the defaults which is off, because we have the prettier plugin.
| // Prevent usage of javascript: in URLs | ||
| 'react/jsx-no-script-url': 'error', | ||
| // Prevent react contexts from taking non-stable values | ||
| 'react/jsx-no-constructed-context-values': 'error', |
| 'error', | ||
| { | ||
| checkFragmentShorthand: true, | ||
| checkKeyMustBeforeSpread: true, |
There was a problem hiding this comment.
We want people to override keys?
There was a problem hiding this comment.
We want to prevent them from overriding a key. To prevent createElement fallback as seen here:
facebook/react#20031 (comment)
This is a major release (breaking change). Which is why I thought the new rules were ok in this. |
Description
This PR updates the
eslint-pluginsand drops support foreslint@6.eslint-config-prettier6.14.07.2.0eslint-plugin-jest-formatting2.0.02.0.1eslint-plugin-promise4.2.14.3.1eslint-plugin-jest24.1.024.1.3eslint-plugin-react7.21.57.22.0eslint-plugin-sort-class-members1.8.01.9.0@typescript-eslint/eslint-plugin4.1.04.15.0@typescript-eslint/parser4.1.04.15.0change-case4.1.04.1.2This PR also enables and disables new rules.
I disabled
arrow-body-styleandprefer-arrow-callback, because they defaulted to off before and are turned off by default inprettier-config/recommendedI 🎩 ed this by using
tophat eslint-plugin ../web.Note web has a package resolution for eslint-plugin-react that you will need to remove first.
Then I ran
Type of change
@shopify/eslint-pluginMajor: Breaking change (fix or feature that would cause existing functionality to not work as expected)Checklist