[#174416949] ESLint config #1

BurnedMarshal · 2020-12-17T16:38:39Z

Porting TSLint rules configuration from io-tslint-rules to ESLint and merged with io-pay ESLint configuration .

The following TSLint rules are not supported for eslint at the moment:

bool-param-default
max-union-size
no-accessor-field-mismatch
no-array-delete // Mitigated by no-delete
no-case-with-or
no-dead-store
no-duplicate-in-composite
no-empty-array
no-extra-semicolon // Mitigated by prettier
no-empty-destructuring // Mititgated by no-empty-pattern
no-gratuitous-expressions
no-hardcoded-credentials
no-ignored-initial-value // Mitigated by no-param-reassign, no-let
no-in-misuse
no-inferred-empty-object-type
no-invalid-await // Mititgated by await-thenable
no-invariant-return
no-misleading-array-reverse // Mitigated by immutable-data
no-misspelled-operator // Mitigated by prettier
no-nested-switch
no-nested-template-literals
no-statements-same-line // Mitigated by prettier
no-try-promise
no-tslint-disable-all
no-unconditional-jump
no-undefined-argument
no-unenclosed-multiline-block // Mitigated by prettier
no-unthrown-error
no-unused-array // Mitigated by no-unused-vars
no-useless-increment
no-useless-intersection
prefer-promise-shorthand
promise-must-complete
use-primitive-type

CODEOWNERS

Co-authored-by: Danilo Spinelli <[email protected]>

…nt-rules into 174416949-eslint-repo-config

AleDore

LGTM apart from minor tuning

AleDore · 2020-12-21T13:02:13Z

recommended.js

+    "functional/immutable-data": "error",
+    "sonarjs/no-small-switch": "off",
+    "sonarjs/no-duplicate-string": "error",
+    "sonarjs/cognitive-complexity": ["error", 19],


We must check on TSLint what is the current cap on cognitive complexity

The default value is 15, we can use it and eventually tuning the value at the project level.

gunzip

can you add a readme.md with the user manual ? (how to integrate in new projects and how to migrate from tslint in existing ones)

gunzip · 2020-12-21T13:01:38Z

package.json

@@ -0,0 +1,34 @@
+{
+  "name": "io-eslint-rules",


why the package and repo names diverge?

The name of the package must be @pagopa/eslint-config to work properly. See this ESLint documentation reference. If we want we can rename the repo to eslint-config. Fixed in 7dd1a75

gunzip · 2020-12-21T13:03:44Z

tsconfig.json

+
+    /* Basic Options */
+    // "incremental": true,                   /* Enable incremental compilation */
+    "target": "es5",                          /* Specify ECMAScript target version: 'ES3' (default), 'ES5', 'ES2015', 'ES2016', 'ES2017', 'ES2018', 'ES2019', 'ES2020', or 'ESNEXT'. */


why not es6?

See #1 (comment)

gunzip · 2020-12-21T13:04:08Z

tsconfig.json

+    // "incremental": true,                   /* Enable incremental compilation */
+    "target": "es5",                          /* Specify ECMAScript target version: 'ES3' (default), 'ES5', 'ES2015', 'ES2016', 'ES2017', 'ES2018', 'ES2019', 'ES2020', or 'ESNEXT'. */
+    "module": "commonjs",                     /* Specify module code generation: 'none', 'commonjs', 'amd', 'system', 'umd', 'es2015', 'es2020', or 'ESNext'. */
+    "lib": ["es2015"],                             /* Specify library files to be included in the compilation. */


See #1 (comment)

gunzip · 2020-12-21T13:04:32Z

tsconfig.json

@@ -0,0 +1,70 @@
+{


why do we need a tsconfig.json here?

It was added for local testing inside this project. We can safely remove it. Fixed in 1a44979

BurnedMarshal · 2020-12-21T13:50:02Z

can you add a readme.md with the user manual ? (how to integrate in new projects and how to migrate from tslint in existing ones)

@gunzip A small README content was added in 32366af and 96e3e9e

.gitignore

.prettierrc.js

Co-authored-by: Emanuele De Cupis <[email protected]>

balanza

LGTM apart minor comments

balanza · 2020-12-21T15:02:55Z

recommended.js

+    "object-shorthand": "error",
+    "no-var": "error",
+    "no-param-reassign": "error",
+    "no-underscore-dangle": "error",


I'm not sure if it would break our convention of prefixing unused params with underscore. In case, we can add

Suggested change

"no-underscore-dangle": "error",

"no-underscore-dangle": ["error", { "allowFunctionParams": true }],

See https://eslint.org/docs/rules/no-underscore-dangle#options

It is, "allowFunctionParams": true (default) allows dangling underscores in function parameter names

So we want it or not? I think we want to be able to write

const fn = (_p) => {}

Sorry, maybe my comment wasn't clear. We can write const fn = (_p) => {}, the option allowFunctionParams has true as default value.

balanza · 2020-12-21T15:05:12Z

recommended.js

+    complexity: "error",
+    "arrow-body-style": "error",
+    "import/no-internal-modules": "off",
+    "import/order": "error",


I'm fine in having this, but I question if it's really useful

balanza · 2020-12-21T15:10:53Z

Great job guys! I'm loving this <3

…nt-rules into 174416949-eslint-repo-config

package.json

Co-authored-by: Emanuele De Cupis <[email protected]>

balanza · 2021-01-07T14:31:37Z

package.json

@@ -1,5 +1,5 @@
 {
-  "name": "eslint-config",
+  "name": "@pagopa/eslint-config",


balanza · 2021-01-07T14:34:15Z

azure-pipelines.yml

+steps:
+  - task: Npm@1
+    inputs:
+      command: 'publish'
+      workingDir: '.'
+      publishEndpoint: 'NPM Registry'
+      verbose: true


this won't work as per this issue microsoft/azure-pipelines-tasks#14165
However we implemented a workaround here pagopa/io-functions-commons#131

I suggest you to do it in another PR in which you can refactor the whole pipeline

[#174416949] Import eslint rules from io-pay

4b04bd3

BurnedMarshal requested review from francescopersico and gunzip and removed request for gunzip and francescopersico December 17, 2020 16:39

[#174416949] remove index.ts testfile

36b677e

gunzip reviewed Dec 17, 2020

View reviewed changes

CODEOWNERS Outdated Show resolved Hide resolved

BurnedMarshal and others added 6 commits December 17, 2020 17:45

Update CODEOWNERS

863ff15

Co-authored-by: Danilo Spinelli <[email protected]>

[#174416949] Porting compatible rules from io-backend

a591c61

Merge branch '174416949-eslint-repo-config' of github.com:pagopa/esli…

a4e6b21

…nt-rules into 174416949-eslint-repo-config

[#174416949] Small fixes remove useless no-extra-parens

edd7a71

[#174416949] add no-delete no-inverted-boolean-check, fix array-type

8b916cf

[#174416949] Fix no-unused-vars whitelist pattern

dee7043

BurnedMarshal marked this pull request as ready for review December 21, 2020 12:22

BurnedMarshal requested review from gunzip, AleDore, cloudify, francescopersico and balanza December 21, 2020 12:22

AleDore reviewed Dec 21, 2020

View reviewed changes

gunzip reviewed Dec 21, 2020

View reviewed changes

BurnedMarshal added 4 commits December 21, 2020 14:14

[#174416949] Remove files for local testing and develop

1a44979

[#174416949] Fix package name

7dd1a75

[#174416949] Add basic README content

32366af

[#174416949] Add italia-tslint-rules reference inside README

96e3e9e

[#174416949] Remove tslint and useless tslint plugins

2cef1e6

gunzip approved these changes Dec 21, 2020

View reviewed changes

balanza reviewed Dec 21, 2020

View reviewed changes

.gitignore Outdated Show resolved Hide resolved

balanza reviewed Dec 21, 2020

View reviewed changes

.prettierrc.js Outdated Show resolved Hide resolved

BurnedMarshal and others added 2 commits December 21, 2020 15:58

Update .gitignore

fa3d72c

Co-authored-by: Emanuele De Cupis <[email protected]>

Update .prettierrc.js

d7d93ee

Co-authored-by: Emanuele De Cupis <[email protected]>

balanza reviewed Dec 21, 2020

View reviewed changes

BurnedMarshal added 2 commits December 21, 2020 16:14

[#174416949] Fix remove unused eslint plugin

129dd25

Merge branch '174416949-eslint-repo-config' of github.com:pagopa/esli…

78618ea

…nt-rules into 174416949-eslint-repo-config

balanza reviewed Dec 21, 2020

View reviewed changes

package.json Outdated Show resolved Hide resolved

BurnedMarshal and others added 3 commits December 21, 2020 16:51

Fix package name with org scope

8c850d1

Co-authored-by: Emanuele De Cupis <[email protected]>

[#174416949] Azure devops pipeline config

2424fae

[#174416949] Add new line

2fb689a

balanza approved these changes Jan 7, 2021

View reviewed changes

balanza merged commit 02ef13a into master Jan 7, 2021

BurnedMarshal mentioned this pull request Jan 12, 2021

Improve README with TSLint migration info #3

Merged

6 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[#174416949] ESLint config #1

[#174416949] ESLint config #1

BurnedMarshal commented Dec 17, 2020 •

edited

Loading

AleDore left a comment

AleDore Dec 21, 2020

BurnedMarshal Dec 21, 2020

gunzip left a comment

gunzip Dec 21, 2020

BurnedMarshal Dec 21, 2020

gunzip Dec 21, 2020

BurnedMarshal Dec 21, 2020

gunzip Dec 21, 2020

BurnedMarshal Dec 21, 2020

gunzip Dec 21, 2020

BurnedMarshal Dec 21, 2020

BurnedMarshal commented Dec 21, 2020

balanza left a comment

balanza Dec 21, 2020

BurnedMarshal Dec 21, 2020

balanza Dec 21, 2020

BurnedMarshal Dec 21, 2020

balanza Dec 21, 2020

balanza commented Dec 21, 2020

balanza Jan 7, 2021

balanza Jan 7, 2021

	"no-underscore-dangle": "error",
	"no-underscore-dangle": ["error", { "allowFunctionParams": true }],

[#174416949] ESLint config #1

[#174416949] ESLint config #1

Conversation

BurnedMarshal commented Dec 17, 2020 • edited Loading

AleDore left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gunzip left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

BurnedMarshal commented Dec 21, 2020

balanza left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

balanza commented Dec 21, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

BurnedMarshal commented Dec 17, 2020 •

edited

Loading