[#IP-86] tslint to eslint migration #139
Conversation
Example of PR titles that include pivotal stories:
New dependencies added: @pagopa/eslint-configAuthor: Unknown Description: This package provide the following ESLint custom rules for Typescript projects. Homepage: https://github.com/pagopa/eslint-rules#readme
|
| Created | about 4 years ago |
| Last Updated | 3 months ago |
| License | MIT |
| Maintainers | 3 |
| Releases | 25 |
| Direct Dependencies | prettier-linter-helpers |
| Keywords | eslint, eslintplugin, eslint-plugin and prettier |
README
eslint-plugin-prettier 
Runs Prettier as an ESLint rule and reports differences as individual ESLint issues.
If your desired formatting does not match Prettier’s output, you should use a different tool such as prettier-eslint instead.
Sample
error: Insert `,` (prettier/prettier) at pkg/commons-atom/ActiveEditorRegistry.js:22:25:
20 | import {
21 | observeActiveEditorsDebounced,
> 22 | editorChangesDebounced
| ^
23 | } from './debounced';;
24 |
25 | import {observableFromSubscribeFunction} from '../commons-node/event';
error: Delete `;` (prettier/prettier) at pkg/commons-atom/ActiveEditorRegistry.js:23:21:
21 | observeActiveEditorsDebounced,
22 | editorChangesDebounced
> 23 | } from './debounced';;
| ^
24 |
25 | import {observableFromSubscribeFunction} from '../commons-node/event';
26 | import {cacheWhileSubscribed} from '../commons-node/observable';
2 errors found.
./node_modules/.bin/eslint --format codeframe pkg/commons-atom/ActiveEditorRegistry.js(code from nuclide).
Installation
npm install --save-dev eslint-plugin-prettier
npm install --save-dev --save-exact prettiereslint-plugin-prettier does not install Prettier or ESLint for you. You must install these yourself.
Then, in your .eslintrc.json:
{
"plugins": ["prettier"],
"rules": {
"prettier/prettier": "error"
}
}Recommended Configuration
This plugin works best if you disable all other ESLint rules relating to code formatting, and only enable rules that detect potential bugs. (If another active ESLint rule disagrees with prettier about how code should be formatted, it will be impossible to avoid lint errors.) You can use eslint-config-prettier to disable all formatting-related ESLint rules.
This plugin ships with a plugin:prettier/recommended config that sets up both the plugin and eslint-config-prettier in one go.
-
In addition to the above installation instructions, install
eslint-config-prettier:npm install --save-dev eslint-config-prettier
-
Then you need to add
plugin:prettier/recommendedas the last extension in your.eslintrc.json:{ "extends": ["plugin:prettier/recommended"] }You can then set Prettier's own options inside a
.prettierrcfile. -
Some ESLint plugins (such as eslint-plugin-react) also contain rules that conflict with Prettier. Add extra exclusions for the plugins you use like so:
{ "extends": [ "plugin:prettier/recommended", "prettier/flowtype", "prettier/react" ] }For the list of every available exclusion rule set, please see the readme of eslint-config-prettier.
Exactly what does plugin:prettier/recommended do? Well, this is what it expands to:
{
"extends": ["prettier"],
"plugins": ["prettier"],
"rules": {
"prettier/prettier": "error",
"arrow-body-style": "off",
"prefer-arrow-callback": "off"
}
}"extends": ["prettier"]enables the main config fromeslint-config-prettier, which turns off some ESLint core rules that conflict with Prettier."plugins": ["prettier"]registers this plugin."prettier/prettier": "error"turns on the rule provided by this plugin, which runs Prettier from within ESLint."arrow-body-style": "off"and"prefer-arrow-callback": "off"turns off two ESLint core rules that unfortunately are problematic with this plugin – see the next section.
arrow-body-style and prefer-arrow-callback issue
If you use arrow-body-style or prefer-arrow-callback together with the prettier/prettier rule from this plugin, you can in some cases end up with invalid code due to a bug in ESLint’s autofix – see issue #65.
For this reason, it’s recommended to turn off these rules. The plugin:prettier/recommended config does that for you.
You can still use these rules together with this plugin if you want, because the bug does not occur all the time. But if you do, you need to keep in mind that you might end up with invalid code, where you manually have to insert a missing closing parenthesis to get going again.
If you’re fixing large of amounts of previously unformatted code, consider temporarily disabling the prettier/prettier rule and running eslint --fix and prettier --write separately.
Options
Note: While it is possible to pass options to Prettier via your ESLint configuration file, it is not recommended because editor extensions such as
prettier-atomandprettier-vscodewill read.prettierrc, but won't read settings from ESLint, which can lead to an inconsistent experience.
-
The first option:
-
An object representing options that will be passed into prettier. Example:
"prettier/prettier": ["error", {"singleQuote": true, "parser": "flow"}]
NB: This option will merge and override any config set with
.prettierrcfiles
-
-
The second option:
-
An object with the following options
-
usePrettierrc: Enables loading of the Prettier configuration file, (default:true). May be useful if you are using multiple tools that conflict with each other, or do not wish to mix your ESLint settings with your Prettier configuration."prettier/prettier": ["error", {}, { "usePrettierrc": false }]
-
fileInfoOptions: Options that are passed to prettier.getFileInfo to decide whether a file needs to be formatted. Can be used for example to opt-out from ignoring files located innode_modulesdirectories."prettier/prettier": ["error", {}, { "fileInfoOptions": { "withNodeModules": true } }]
-
-
-
The rule is autofixable -- if you run
eslintwith the--fixflag, your code will be formatted according toprettierstyle.
Contributing
See CONTRIBUTING.md
Codecov Report
@@ Coverage Diff @@
## master #139 +/- ##
==========================================
+ Coverage 84.43% 85.23% +0.80%
==========================================
Files 33 33
Lines 880 867 -13
Branches 86 91 +5
==========================================
- Hits 743 739 -4
+ Misses 134 125 -9
Partials 3 3
Continue to review full report at Codecov.
|
michaeldisaro
requested review from
AleDore,
balanza,
BurnedMarshal,
francescopersico and
gunzip
as code owners
michaeldisaro
changed the title
There was a problem hiding this comment.
Changes look good to me, however I'm seeing an over-use of escaping for @typescript-eslint/naming-convention. That may mean the rule doesn't fit well with out code style, we may want to review it in the @pagopa/eslint-rules package
Thoughts @BurnedMarshal?
|
I think it would be better to change code style not the rule. I found too
much incoherence in naming.
Il ven 9 apr 2021, 22:58 Emanuele De Cupis ***@***.***> ha
scritto:
… ***@***.**** commented on this pull request.
Changes look good to me, however I'm seeing an over-use of escaping for
@typescript-eslint/naming-convention. That may mean the rule doesn't fit
well with out code style, we may want to review it in the
@pagopa/eslint-rules package
Thoughts @BurnedMarshal <https://github.com/BurnedMarshal>?
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#139 (review)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ADT45DQ2XGU2UTVUXWWKLUDTH5TALANCNFSM42OTWM3Q>
.
|
@balanza we can extend the default rule for "@typescript-eslint/naming-convention": [
"error",
// DEFAULT STRICT camelCase RULES
{
selector: 'default',
format: ['camelCase'],
leadingUnderscore: 'allow',
trailingUnderscore: 'allow',
},
{
selector: 'variable',
format: ['camelCase', 'UPPER_CASE'],
leadingUnderscore: 'allow',
trailingUnderscore: 'allow',
},
{
selector: 'typeLike',
format: ['PascalCase'],
},
// EXPORTED io-ts TYPES are PascalCase
{
"selector": "variable",
"format": ["PascalCase", "camelCase", "UPPER_CASE"],
"modifiers": ["exported"]
},
// LOCAL io-ts TYPES (not exported)
{
"selector": "variable",
"format": ["PascalCase", "UPPER_CASE"],
"filter": {
regex: "^.{1,}[RO]$",
match: true
}
}
]We should evaluate if we have different naming convention across several project type (functions, backend). |
|
That's the way, I think. However, we must refine such definition as the following would still throw: export const ValidationToken = t.interface({
Email: EmailAddress,
FiscalCode,
InvalidAfter: UTCISODateFromString
});We must include object fields as selector |
|
Commit ddcf4ca broke the linter intentionally - we wait for pagopa/eslint-rules#5 and pagopa/eslint-rules#6 to be merged and than we will inherit such rules from the central package. |
List of Changes
tslint.jsonfilepackage.json.eslintrc.jsconfiguration.eslintrc.jsto avoid a deep refactoring of working codeMotivation and Context
We want to migrate to eslint because tslint is deprecated.
How Has This Been Tested?
It has been tested by performing:
Types of changes
Checklist: