feat(build): add eslint scripts and default configs #2572

raymondfeng · 2019-03-11T18:33:00Z

Phase 1 for #2492:

Add @loopback/eslint-config
Add lb-eslint to @loopback/build

Checklist

👉 Read and sign the CLA (Contributor License Agreement) 👈

npm test passes on your machine
New tests added or existing tests modified to cover all changes
Code conforms with the style guide
API Documentation in code was updated
Documentation in /docs/site was updated
Affected artifact templates in packages/cli were updated
Affected example projects in examples/* were updated

bajtos · 2019-03-12T09:45:07Z

Yay for starting with a smaller patch first 👍 ❤️

bajtos · 2019-03-12T09:47:37Z

packages/eslint-config/.eslintrc.js

+    '@typescript-eslint/prefer-interface': 'off',
+    '@typescript-eslint/no-namespace': 'off',
+    '@typescript-eslint/no-unused-vars': 'off',
+    '@typescript-eslint/ban-types': 'error',


As discussed in #2492, our code base is violating ban-types rule in many places. Let's disable this rule in this initial pull request to make the migration from tslint to eslint easier & faster.

packages/eslint-config/.npmrc

packages/eslint-config/README.md

packages/eslint-config/package.json

packages/eslint-config/.eslintrc.js

greenkeeper.json

.eslintignore

.travis.yml

bajtos

This comment is still not addressed: #2572 (comment)

Would it help you @raymondfeng if I took a look at the differences between tslint and your proposed config myself?

bajtos · 2019-03-29T15:57:11Z

package.json

+    "eslint": "^5.14.1",
+    "eslint-config-prettier": "^4.1.0",
+    "eslint-plugin-eslint-plugin": "^2.0.1",
+    "eslint-plugin-mocha": "^5.3.0",


I think these new dependencies at root level should not be needed as long as we always use the eslint config bundled as part of @loopback/build. Could you please try to remove them?

I think these new dependencies at root level should not be needed as long as we always use the eslint config bundled as part of @loopback/build. Could you please try to remove them?

☝️

raymondfeng · 2019-03-29T19:49:57Z

@bajtos I'm busy with other PRs and will visit this one again soon.

raymondfeng · 2019-05-22T15:18:27Z

@bajtos I have updated ESLint configuration to reflect existing tslint rules.

bajtos

Looks mostly good, please address my minor comments below before landing.

I didn't test this new PR in practice, I am considering this PR as an initial version that will be incrementally improved in follow-up pull requests as needed.

bajtos · 2019-05-24T12:54:02Z

package.json

+    "eslint": "^5.14.1",
+    "eslint-config-prettier": "^4.1.0",
+    "eslint-plugin-eslint-plugin": "^2.0.1",
+    "eslint-plugin-mocha": "^5.3.0",


I think these new dependencies at root level should not be needed as long as we always use the eslint config bundled as part of @loopback/build. Could you please try to remove them?

☝️

bajtos · 2019-05-24T12:55:45Z

packages/eslint-config/eslintrc.js

+    sourceType: 'module',
+    project: './tsconfig.json',
+    ecmaFeatures: {
+      ecmaVersion: 2017,


I believe we are using 2018, see https://github.com/strongloop/loopback-next/blob/75939a4a144b3068ff2890394c178faad58d7458/packages/build/config/tsconfig.common.json#L13

bajtos · 2019-05-24T12:58:12Z

packages/eslint-config/eslintrc.js

+    '@typescript-eslint/adjacent-overload-signatures': 'error', // tslint:adjacent-overload-signatures
+    '@typescript-eslint/prefer-for-of': 'error', // tslint:prefer-for-of
+    '@typescript-eslint/unified-signatures': 'error', // tslint:unified-signatures
+    '@typescript-eslint/no-explicit-any': 'error', // tslint:no-explicit-any


At the moment, we are using no-any rule, can you enable it too please? Or is @typescript-eslint/no-explicit-any replacement for tslint:no-any and it's just the comment that's wrong here?

See https://github.com/strongloop/loopback-next/blob/4b68ef850318e865074449f2892e4f616aebaa0c/packages/tslint-config/tslint.common.json#L12

bajtos · 2019-05-24T13:07:09Z

packages/eslint-config/eslintrc.js

+    '@typescript-eslint/await-thenable': 'error', // tslint:await-promise: [true, 'PromiseLike', 'RequestPromise'],
+
+    // https://github.com/xjamundx/eslint-plugin-promise
+    // tslint:no-floating-promises: [true, 'PromiseLike', 'RequestPromise'],


Hmm, no-floating-promise is actually very useful! It detects situations where people forget to await a promise.

Here is the tracking issue: typescript-eslint/typescript-eslint#464 and pull request in progress: typescript-eslint/typescript-eslint#495

Please rework the comment to point to those two links instead of https://github.com/xjamundx/eslint-plugin-promise.

I am ok to move forward with this PR even if no-floating-promises is not supported yet.

bajtos · 2019-05-24T13:09:06Z

packages/eslint-config/eslintrc.js

+};
+
+/**
+ * tslint rules from `@loopback/tslint-config` for comparison


Let's remove the commented out code (L115-EOF). For anybody reviewing this patch, here are links to GitHub view of those two files:

tslint.common.json

https://github.com/strongloop/loopback-next/blob/4b68ef850318e865074449f2892e4f616aebaa0c/packages/tslint-config/tslint.common.json#L7-L30

tslint.build.json
https://github.com/strongloop/loopback-next/blob/4b68ef850318e865074449f2892e4f616aebaa0c/packages/tslint-config/tslint.build.json#L9-L23

See https://github.com/typescript-eslint/typescript-eslint

raymondfeng requested a review from bajtos as a code owner March 11, 2019 18:33

raymondfeng mentioned this pull request Mar 11, 2019

feat: introduce eslint with typescript-eslint to replace tslint #2492

Merged

7 tasks

raymondfeng force-pushed the eslint-1 branch 2 times, most recently from 91dab0c to b56849a Compare March 12, 2019 03:45

bajtos suggested changes Mar 12, 2019

View reviewed changes

bajtos added feature Internal Tooling Issues related to our tooling and monorepo infrastructore labels Mar 12, 2019

bajtos reviewed Mar 12, 2019

View reviewed changes

greenkeeper.json Show resolved Hide resolved

raymondfeng force-pushed the eslint-1 branch 9 times, most recently from f9d62b8 to d17ee28 Compare March 14, 2019 17:17

bajtos mentioned this pull request Mar 18, 2019

Switch from tslint to eslint #2608

Closed

bajtos reviewed Mar 21, 2019

View reviewed changes

.eslintignore Outdated Show resolved Hide resolved

.travis.yml Outdated Show resolved Hide resolved

raymondfeng force-pushed the eslint-1 branch from d17ee28 to d73c4c3 Compare March 21, 2019 16:56

raymondfeng requested a review from bajtos March 21, 2019 17:12

raymondfeng force-pushed the eslint-1 branch from d73c4c3 to b5537de Compare March 26, 2019 21:52

bajtos reviewed Mar 29, 2019

View reviewed changes

raymondfeng force-pushed the eslint-1 branch from b5537de to 501cad6 Compare April 17, 2019 20:16

bajtos mentioned this pull request May 2, 2019

feat: add lb3 example application #2803

Merged

7 tasks

raymondfeng force-pushed the eslint-1 branch 3 times, most recently from 9c53e9b to 0da00ea Compare May 22, 2019 15:17

bajtos approved these changes May 24, 2019

View reviewed changes

raymondfeng force-pushed the eslint-1 branch from 0da00ea to b46f3e8 Compare May 28, 2019 16:57

feat(build): add eslint scripts and default configs

fbeb150

See https://github.com/typescript-eslint/typescript-eslint

raymondfeng force-pushed the eslint-1 branch from b46f3e8 to 1288d3d Compare May 28, 2019 20:55

chore: enable eslint for loopback-next monorepo

8452146

raymondfeng force-pushed the eslint-1 branch from 1288d3d to 8452146 Compare May 28, 2019 22:20

raymondfeng merged commit 10259b2 into master May 28, 2019

b-admike deleted the eslint-1 branch May 29, 2019 22:18

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(build): add eslint scripts and default configs #2572

feat(build): add eslint scripts and default configs #2572

raymondfeng commented Mar 11, 2019 •

edited

bajtos commented Mar 12, 2019

bajtos Mar 12, 2019

bajtos left a comment

bajtos Mar 29, 2019

bajtos May 24, 2019

raymondfeng commented Mar 29, 2019

raymondfeng commented May 22, 2019

bajtos left a comment

bajtos May 24, 2019

bajtos May 24, 2019

bajtos May 24, 2019

bajtos May 24, 2019

bajtos May 24, 2019

feat(build): add eslint scripts and default configs #2572

feat(build): add eslint scripts and default configs #2572

Conversation

raymondfeng commented Mar 11, 2019 • edited

Checklist

bajtos commented Mar 12, 2019

Choose a reason for hiding this comment

bajtos left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

raymondfeng commented Mar 29, 2019

raymondfeng commented May 22, 2019

bajtos 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

raymondfeng commented Mar 11, 2019 •

edited