chore(code-lint): Add lint to check (VSCODE-215) #230

Anemy · 2021-01-04T06:21:28Z

This PR adds a lint check for our check script, which runs in the precommit. Updated our eslint to search all the files, and added all of our entries to the dependency check which mongodb-js-precommit runs.

This PR includes fixes for the existing lint errors, and some of the errors. There are still a couple warnings (see screenshot below). Also updates a couple of our dependencies and our eslint config generation to be a bit cleaner and work with the new deps.

Here's what the commit output now looks like:

Looks like mongodb-js-precommit is throwing en error when trying to lint now. I think we want to add an optional parameter to skip the lint check. Haven't looked into what's causing it to fail yet.

src/editors/editorsController.ts

mcasimir · 2021-01-04T08:55:51Z

Error: Callback was already called is haunting VSCode too :)

package.json

src/editors/editorsController.ts

mcasimir

Nice! Maybe Alena wants to review this too.

alenakhineika · 2021-01-04T11:03:08Z

src/explorer/playgroundsTree.ts

+      return vscode.FileType.SymbolicLink;
+    }
+
+    return vscode.FileType.Unknown;


src/language/server.ts

src/language/visitor.ts

alenakhineika · 2021-01-04T11:05:38Z

src/language/worker.ts

-        }))
+        label: item.name,
+        kind: CompletionItemKind.Value
+      }))


Will this endless adding/deleting spaces be fixed by this PR with lint improvements?

With #231 yes

src/telemetry/telemetryController.ts

Anemy · 2021-01-04T13:19:27Z

@mcasimir Removed the mongodb-js-precommit dependency and added depcheck. Looks like it's doing a better job than dependency-check was doing too, updated package.json to remove a few unused deps and explicitly add download and fs-extra which we didn't have. Here's what the precommit output looks like now:

src/editors/editorsController.ts

alenakhineika · 2021-01-05T16:19:14Z

package.json

    "@types/ws": "^7.2.4",
-    "@typescript-eslint/eslint-plugin": "^2.19.2",
-    "@typescript-eslint/parser": "^2.19.2",
+    "@typescript-eslint/eslint-plugin": "^4.11.1",


This and "@typescript-eslint/parser" are already 4.12.0

👍 will rebase once the editing playground results pr goes through and then we can look at this again and merge.

Anemy · 2021-01-11T11:38:51Z

Add https://github.com/typescript-eslint/typescript-eslint/blob/master/packages/eslint-plugin/src/configs/recommended-requiring-type-checking.ts#L21

…e can skip lint

…rrors

Anemy · 2021-01-12T13:15:55Z

@alenakhineika @mcasimir rebased off master, fixed those issues. Also added parser: '@typescript-eslint/parser' and 'plugin:@typescript-eslint/recommended-requiring-type-checking' to the eslint config. They introduce a ton of warnings, but they all look like good warnings. I think we can reduce them overtime. Want to try them out and see if there are any we should change? We can also do in future prs if better too.

Something I noticed in a number of our tests is we're importing sinon and sometimes other packages as any types which could lead to some misuse/broken tests. I'll address those in a later pr. We could also definitely reduce the amount of any in this project a lot lol.

Anemy requested a review from alenakhineika January 4, 2021 06:21

Anemy commented Jan 4, 2021

View reviewed changes

src/editors/editorsController.ts Outdated Show resolved Hide resolved

Anemy changed the title ~~chore: Add lint to check~~ chore(code-lint): Add lint to check Jan 4, 2021