-
-
Notifications
You must be signed in to change notification settings - Fork 278
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
Update to eslint flat config and yea/nay all rules #3240
Commits on Jul 16, 2024
-
Moving eslint config to flat config system
This is the default for eslint 9 and will be the only option in eslint 10. See https://eslint.org/docs/latest/use/configure/migration-guide There's apparently a bug with the globals release we had automatically installed and eslint flat config, so just updated to the latest version manually since that one is fixed.
Configuration menu - View commit details
-
Copy full SHA for c02e844 - Browse repository at this point
Copy the full SHA c02e844View commit details -
Fix previously undetected eslint issues
For some reason (.mjs differences?) these two files had eslint issues that were not reported with the old config.
Configuration menu - View commit details
-
Copy full SHA for 7e75318 - Browse repository at this point
Copy the full SHA 7e75318View commit details -
Move unfixed rules into the flat eslint config
Rather than having a convoluted system to check unfixed rules, this just has them apply to all files, and only selectively disables them for files we know to be unfixed. This helps us not introduce more issues in the meantime. I did generalize a bit for release-editor and similar folders where we don't expect to fix the files but convert them entirely, so there's no point fixing them file by file.
Configuration menu - View commit details
-
Copy full SHA for 30ab720 - Browse repository at this point
Copy the full SHA 30ab720View commit details -
Fix previously undetected eslint issues
For some reason (.mjs differences?) these files had eslint issues that were not reported with the old config.
Configuration menu - View commit details
-
Copy full SHA for 490f177 - Browse repository at this point
Copy the full SHA 490f177View commit details -
Add no-constant-binary-expression rule
This is a new recommended rule in eslint 9: https://eslint.org/docs/latest/use/migrate-to-9.0.0
Configuration menu - View commit details
-
Copy full SHA for 42bd47a - Browse repository at this point
Copy the full SHA 42bd47aView commit details -
Use @Stylistic JS eslint plugin
Most stylistic rules have been split from the eslint package and moved to a separate one - the eslint ones still work but are officially deprecated, so we might as well update before they break. I'll move these to a separate block in a second commit to make it easier to review.
Configuration menu - View commit details
-
Copy full SHA for 55299d4 - Browse repository at this point
Copy the full SHA 55299d4View commit details -
Configuration menu - View commit details
-
Copy full SHA for 7bc2043 - Browse repository at this point
Copy the full SHA 7bc2043View commit details -
Configuration menu - View commit details
-
Copy full SHA for 3044041 - Browse repository at this point
Copy the full SHA 3044041View commit details -
https://eslint.style/rules/js/dot-location We consistently use the property dot location position. Especially given how that's not even the eslint default, it seems worth enforcing with a rule.
Configuration menu - View commit details
-
Copy full SHA for 0a5f600 - Browse repository at this point
Copy the full SHA 0a5f600View commit details -
Add eslint generator-star-spacing rule
https://eslint.style/rules/js/generator-star-spacing We consistently use the 'after' style. Especially given how that's not even the eslint default, it seems worth enforcing with a rule.
Configuration menu - View commit details
-
Copy full SHA for 36580f9 - Browse repository at this point
Copy the full SHA 36580f9View commit details -
Use stylistic plugin no-extra-semi
This was listed under errors, but it's a stylistic rule.
Configuration menu - View commit details
-
Copy full SHA for fb7d769 - Browse repository at this point
Copy the full SHA fb7d769View commit details -
Use stylistic plugin no-floating-decimal
This was listed under best practices, but it's a stylistic rule.
Configuration menu - View commit details
-
Copy full SHA for 917e7e5 - Browse repository at this point
Copy the full SHA 917e7e5View commit details -
Use stylistic plugin no-multi-spaces
This was listed under best practices, but it's a stylistic rule.
Configuration menu - View commit details
-
Copy full SHA for 65ba677 - Browse repository at this point
Copy the full SHA 65ba677View commit details -
Use stylistic plugin rest-spread-spacing
This was listed under ECMAScript 6, but it's a stylistic rule.
Configuration menu - View commit details
-
Copy full SHA for 23f8f88 - Browse repository at this point
Copy the full SHA 23f8f88View commit details -
Use stylistic plugin template-curly-spacing
This was listed under ECMAScript 6, but it's a stylistic rule.
Configuration menu - View commit details
-
Copy full SHA for 401adb0 - Browse repository at this point
Copy the full SHA 401adb0View commit details -
Add eslint yield-star-spacing rule
https://eslint.style/rules/js/yield-star-spacing We were inconsistent here, but I talked to mwiencek and we both prefer the 'after' default.
Configuration menu - View commit details
-
Copy full SHA for 948b84c - Browse repository at this point
Copy the full SHA 948b84cView commit details -
Use @Stylistic JSX eslint plugin
Most stylistic React rules have been split from the react package and moved to a separate one - the react ones still work but are officially deprecated, so we might as well update before they break. I'll move these to a separate block in a second commit to make it easier to review.
Configuration menu - View commit details
-
Copy full SHA for 1eb74d0 - Browse repository at this point
Copy the full SHA 1eb74d0View commit details -
Configuration menu - View commit details
-
Copy full SHA for 5a3c942 - Browse repository at this point
Copy the full SHA 5a3c942View commit details -
Add eslint jsx-curly-newline rule
https://eslint.style/rules/jsx/jsx-curly-newline We were inconsistent here, but not horribly so. Auto --fix'ed 38 issues.
Configuration menu - View commit details
-
Copy full SHA for 82c98b0 - Browse repository at this point
Copy the full SHA 82c98b0View commit details -
Update no-new-symbol to no-new-native-nonconstructor
The old rule is deprecated in 9.0.0 and replaced with the new one, which is already available, so making the change in preparation. We don't have any clashes with the new rule, it seems.
Configuration menu - View commit details
-
Copy full SHA for 1c18fe1 - Browse repository at this point
Copy the full SHA 1c18fe1View commit details -
Configuration menu - View commit details
-
Copy full SHA for 829a6fd - Browse repository at this point
Copy the full SHA 829a6fdView commit details -
Configuration menu - View commit details
-
Copy full SHA for 7ea7c67 - Browse repository at this point
Copy the full SHA 7ea7c67View commit details -
Apply or actively reject eslint problem solving rules
We already intentionally clarified we did not use a lot of rules. This is the first step into making that more generalized, plus it applies a bunch of newer rules we already followed but might actually want to enforce.
Configuration menu - View commit details
-
Copy full SHA for cfd0af6 - Browse repository at this point
Copy the full SHA cfd0af6View commit details -
Organize rules like eslint does
This was fairly arbitrary anyway, and this makes it easier to compare with the eslint rule docs.
Configuration menu - View commit details
-
Copy full SHA for 8d1d1a7 - Browse repository at this point
Copy the full SHA 8d1d1a7View commit details -
Add eslint default-case-last rule
https://eslint.org/docs/latest/rules/default-case-last We have one use which kinda makes sense, but is also confusing. There's no reason we cannot put case 0 and default last, and we don't actually have a type 0, so it's already a default anyway.
Configuration menu - View commit details
-
Copy full SHA for 5a001f0 - Browse repository at this point
Copy the full SHA 5a001f0View commit details -
Add eslint default-param-last rule
https://eslint.org/docs/latest/rules/default-param-last We have three cases where it seems to be needed to flout the rule (two seem to just be because the rule doesn't understand that ? means the param is optional anyway). Added eslint-disable comments for those.
Configuration menu - View commit details
-
Copy full SHA for 3485cf4 - Browse repository at this point
Copy the full SHA 3485cf4View commit details -
Add eslint no-case-declarations rule
https://eslint.org/docs/latest/rules/no-case-declarations None of our few hits seem intentional (they all end in break;), so just added brackets as suggested by the rule.
Configuration menu - View commit details
-
Copy full SHA for 5be4506 - Browse repository at this point
Copy the full SHA 5be4506View commit details -
Add eslint no-useless-concat rule
https://eslint.org/docs/latest/rules/no-useless-concat The two places where we hit this do not seem more clear for having two separate strings.
Configuration menu - View commit details
-
Copy full SHA for 1520d50 - Browse repository at this point
Copy the full SHA 1520d50View commit details -
Add eslint no-useless-escape rule
https://eslint.org/docs/latest/rules/no-useless-escape We had a somewhat embarrassing amount of hits here, including a few where we were escaping *too little*.
Configuration menu - View commit details
-
Copy full SHA for 4540077 - Browse repository at this point
Copy the full SHA 4540077View commit details -
https://eslint.org/docs/latest/rules/no-redeclare We had one hit, where the redeclared param seems useless anyway.
Configuration menu - View commit details
-
Copy full SHA for 27fc678 - Browse repository at this point
Copy the full SHA 27fc678View commit details -
Add eslint no-return-assign rule
https://eslint.org/docs/latest/rules/no-return-assign Couple of places where we didn't want to return anything anyway, so just doing { }. In URLCleanup, all we gained was one line, so eh.
Configuration menu - View commit details
-
Copy full SHA for 8a0e808 - Browse repository at this point
Copy the full SHA 8a0e808View commit details -
https://eslint.org/docs/latest/rules/no-loop-func The only place where we hit this is actually fine because it's executed immediately but in general this is a good thing to keep in mind.
Configuration menu - View commit details
-
Copy full SHA for 6fcf118 - Browse repository at this point
Copy the full SHA 6fcf118View commit details -
Add eslint logical-assignment-operators rule
https://eslint.org/docs/latest/rules/logical-assignment-operators We weren't using these at all, but it actually seems quite nice and avoids a lot of repetition.
Configuration menu - View commit details
-
Copy full SHA for dea9185 - Browse repository at this point
Copy the full SHA dea9185View commit details -
Add eslint no-implicit-coercion rule
https://eslint.org/docs/latest/rules/no-implicit-coercion In many cases, especially when we have Flow types, it's a lot better to just do the actual check that makes sense based on the data instead of having boolean coercion. For places where we don't have Flow, or coercion to string or number, it's probably sensible to just do the coercion explicitly.
Configuration menu - View commit details
-
Copy full SHA for 956ebd2 - Browse repository at this point
Copy the full SHA 956ebd2View commit details -
https://eslint.org/docs/latest/rules/func-style By far most of our (non-arrow) functions were already using the declaration style, so this just standardises the last few cases for consistency.
Configuration menu - View commit details
-
Copy full SHA for 5eaad03 - Browse repository at this point
Copy the full SHA 5eaad03View commit details -
Add eslint no-useless-return rule
https://eslint.org/docs/latest/rules/no-useless-return We had one case (my own fault).
Configuration menu - View commit details
-
Copy full SHA for 40e1ee6 - Browse repository at this point
Copy the full SHA 40e1ee6View commit details -
Configuration menu - View commit details
-
Copy full SHA for 49f8952 - Browse repository at this point
Copy the full SHA 49f8952View commit details -
Add eslint object-shorthand rule
https://eslint.org/docs/latest/rules/object-shorthand All --fix'ed. In TagEditor Flow was worried but it seems to be a false positive for object-this-reference.
Configuration menu - View commit details
-
Copy full SHA for 7683cba - Browse repository at this point
Copy the full SHA 7683cbaView commit details -
Add eslint prefer-object-has-own rule
https://eslint.org/docs/latest/rules/prefer-object-has-own Using this means we no longer seem to have a need for our own function hasOwnProp.
Configuration menu - View commit details
-
Copy full SHA for 2a3a774 - Browse repository at this point
Copy the full SHA 2a3a774View commit details -
Add eslint prefer-regex-literals rule
https://eslint.org/docs/latest/rules/prefer-regex-literals Almost all changes are in the match section of URLCleanup. Honestly, I'm not sure what the original reason was for that part to be a string regex - there was a slight benefit, in that it allowed having the regex in several lines to make it a bit more readable. That said, there were so many cases where we were over-escaping and, a lot worse, a bunch where we were *under-escaping*, leading to actually incorrect regexes. This way is a lot easier to get the escapes right.
Configuration menu - View commit details
-
Copy full SHA for 5d0cd29 - Browse repository at this point
Copy the full SHA 5d0cd29View commit details -
Configuration menu - View commit details
-
Copy full SHA for fa9b4bb - Browse repository at this point
Copy the full SHA fa9b4bbView commit details -
https://eslint.org/docs/latest/rules/prefer-spread GuessCase/Main was messy enough that this refactors it a bit to make Flow less angry.
Configuration menu - View commit details
-
Copy full SHA for da08136 - Browse repository at this point
Copy the full SHA da08136View commit details -
https://eslint.org/docs/latest/rules/require-await This mostly hit cases where we didn't actually need async since they return Promises. There was one case where we needed to wrap some returns in Promise wrappers, but that's about it.
Configuration menu - View commit details
-
Copy full SHA for 9ec526c - Browse repository at this point
Copy the full SHA 9ec526cView commit details -
Activate the eslint strict rule
We don't need this to remind us to use strict, but it did warn me of one unnecessary use of it! So worth having on for that.
Configuration menu - View commit details
-
Copy full SHA for 00a4ecb - Browse repository at this point
Copy the full SHA 00a4ecbView commit details -
https://eslint.org/docs/latest/rules/yoda Three cases of this we had, a bit stupid it looked.
Configuration menu - View commit details
-
Copy full SHA for 3d0531c - Browse repository at this point
Copy the full SHA 3d0531cView commit details -
Add other Suggestions rules from eslint
This adds some rules that seem useful as warn, and lists all others as off, so we know we looked at them and decided against them rather than not having decided yet.
Configuration menu - View commit details
-
Copy full SHA for 1b9374a - Browse repository at this point
Copy the full SHA 1b9374aView commit details -
Add eslint no-useless-path-segments rule
https://github.com/import-js/eslint-plugin-import/blob/main/docs/rules/no-useless-path-segments.md We had a fair amount of these!
Configuration menu - View commit details
-
Copy full SHA for 77f247e - Browse repository at this point
Copy the full SHA 77f247eView commit details -
Add eslint no-extraneous-dependencies rule
https://github.com/import-js/eslint-plugin-import/blob/main/docs/rules/no-extraneous-dependencies.md We only had one of this, defined in selenium.mjs... which seems to be pretty useless anyway, so just replacing it with a simpler option (we shouldn't really care if the message is '' or undefined).
Configuration menu - View commit details
-
Copy full SHA for a622e58 - Browse repository at this point
Copy the full SHA a622e58View commit details -
Add eslint prefer-default-export rule
https://github.com/import-js/eslint-plugin-import/blob/main/docs/rules/prefer-default-export.md There's no good reason anymore to not have a default export for manifest (we used to have two exports but we no longer do). Other files were a lot more trivial. tableColumns is clearly meant as something that might eventually have more exports, and so is report/constants, so I disabled the rule there. Same for i18n/history.
Configuration menu - View commit details
-
Copy full SHA for 7987429 - Browse repository at this point
Copy the full SHA 7987429View commit details -
Document eslint-plugin-import rule usage
This activates a few more rules and specifies the ones we do not want are intentionally off. There's a few rules that do not currently work with a flat config setup but we probably do want; I commented those out for now.
Configuration menu - View commit details
-
Copy full SHA for f8c485b - Browse repository at this point
Copy the full SHA f8c485bView commit details -
Configuration menu - View commit details
-
Copy full SHA for f468b36 - Browse repository at this point
Copy the full SHA f468b36View commit details -
Configuration menu - View commit details
-
Copy full SHA for 6c3448e - Browse repository at this point
Copy the full SHA 6c3448eView commit details -
Document eslint-plugin-js rule usage
This activates a few more rules and specifies the ones we do not want are intentionally off.
Configuration menu - View commit details
-
Copy full SHA for ec1b295 - Browse repository at this point
Copy the full SHA ec1b295View commit details -
Document eslint-jsx stylistic rule usage
This activates a few more rules and specifies the ones we do not want are intentionally off.
Configuration menu - View commit details
-
Copy full SHA for 300f3d8 - Browse repository at this point
Copy the full SHA 300f3d8View commit details -
Configuration menu - View commit details
-
Copy full SHA for f84795a - Browse repository at this point
Copy the full SHA f84795aView commit details -
Configuration menu - View commit details
-
Copy full SHA for 6886150 - Browse repository at this point
Copy the full SHA 6886150View commit details -
Configuration menu - View commit details
-
Copy full SHA for 7072352 - Browse repository at this point
Copy the full SHA 7072352View commit details -
Document eslint-plugin-react rule usage
This activates a few more rules and specifies the ones we do not want are intentionally off.
Configuration menu - View commit details
-
Copy full SHA for c85e89c - Browse repository at this point
Copy the full SHA c85e89cView commit details -
Add eslint ft-flow/enforce-suppression-code rule
https://github.com/flow-typed/eslint-plugin-ft-flow?tab=readme-ov-file#enforce-suppression-code Our Multiselect typing is a horrible mess that we will fix in a separate PR, for now this disables the rule there since we have to ignore *four* Flow rules for them.
Configuration menu - View commit details
-
Copy full SHA for d5ed41b - Browse repository at this point
Copy the full SHA d5ed41bView commit details -
Configuration menu - View commit details
-
Copy full SHA for f875680 - Browse repository at this point
Copy the full SHA f875680View commit details -
Configuration menu - View commit details
-
Copy full SHA for 736134f - Browse repository at this point
Copy the full SHA 736134fView commit details -
Add eslint ft-flow/no-internal-flow-type rule
https://github.com/flow-typed/eslint-plugin-ft-flow?tab=readme-ov-file#no-internal-flow-type We were using these to avoid importing React all the time, but a recent Flow version made it so that it's no longer needed, so we might as well return to the standard types.
Configuration menu - View commit details
-
Copy full SHA for 5a64d78 - Browse repository at this point
Copy the full SHA 5a64d78View commit details -
Document eslint-plugin-ft-flow rule usage
This activates a few more rules and specifies the ones we do not want are intentionally off.
Configuration menu - View commit details
-
Copy full SHA for 7a5bc2c - Browse repository at this point
Copy the full SHA 7a5bc2cView commit details -
Configuration menu - View commit details
-
Copy full SHA for 73fae4c - Browse repository at this point
Copy the full SHA 73fae4cView commit details -
Configuration menu - View commit details
-
Copy full SHA for 04a0b7c - Browse repository at this point
Copy the full SHA 04a0b7cView commit details -
Document eslint-plugin-fb-flow rule usage
This activates a few more rules and specifies the ones we do not want are intentionally off.
Configuration menu - View commit details
-
Copy full SHA for 0d75ce9 - Browse repository at this point
Copy the full SHA 0d75ce9View commit details -
Configuration menu - View commit details
-
Copy full SHA for cbc62c9 - Browse repository at this point
Copy the full SHA cbc62c9View commit details -
Configuration menu - View commit details
-
Copy full SHA for 504d5ce - Browse repository at this point
Copy the full SHA 504d5ceView commit details -
Configuration menu - View commit details
-
Copy full SHA for 12dc0eb - Browse repository at this point
Copy the full SHA 12dc0ebView commit details
Commits on Jul 17, 2024
-
Update eslint-plugin-ft-flow to v3.0.9
This fixes a bug where flow annotations were not seen when in multiline comments - so most of our files... A lot of issues came up that were previously hidden for 'no-flow-suppressions-in-strict-files'.
Configuration menu - View commit details
-
Copy full SHA for 9f7822e - Browse repository at this point
Copy the full SHA 9f7822eView commit details -
Configuration menu - View commit details
-
Copy full SHA for 1eb0401 - Browse repository at this point
Copy the full SHA 1eb0401View commit details -
Configuration menu - View commit details
-
Copy full SHA for 7022a20 - Browse repository at this point
Copy the full SHA 7022a20View commit details -
Configuration menu - View commit details
-
Copy full SHA for 5c6cbd5 - Browse repository at this point
Copy the full SHA 5c6cbd5View commit details -
Update eslint-plugin-js and -jsx to 2.3.0
https://github.com/eslint-stylistic/eslint-stylistic/releases/tag/v2.2.0 https://github.com/eslint-stylistic/eslint-stylistic/releases/tag/v2.2.1 https://github.com/eslint-stylistic/eslint-stylistic/releases/tag/v2.2.2 https://github.com/eslint-stylistic/eslint-stylistic/releases/tag/v2.3.0 None of the new options seem preferable, so not using them. 2.3.0 deprecates jsx-indent because indent from @stylistic/js does all the same things by now.
Configuration menu - View commit details
-
Copy full SHA for ff02e28 - Browse repository at this point
Copy the full SHA ff02e28View commit details -
Configuration menu - View commit details
-
Copy full SHA for a7fe8c3 - Browse repository at this point
Copy the full SHA a7fe8c3View commit details -
Configuration menu - View commit details
-
Copy full SHA for 8e33e64 - Browse repository at this point
Copy the full SHA 8e33e64View commit details