FEI-4957.9: Update eslint config to use typescript parser and plugins (…

…#531)

## Summary:
It took a while to figure out the magic incantations to get imports to be recognized.  The config right now is split over packages/eslint-khan-config/index.js and .eslintrc.js.  In a separate PR, I'm going to pull stuff that looks like it's not repo-specific out of .eslintrc.js and into eslint-khan-config/index.js.  We can use the latter to help with migrating other codebases.

This PR also needed to make some changes to how we were mocking npm modules in our tests.  TBH, I'm not quite sure what changed in this PR that precipitated the need to make this change.  Previous PRs in the stack were running the tests and the test were passing fine then.  It may have been the change of some imports from default imports to namespace imports.

Issue: FEI-4957

## Test plan:
- yarn lint

Author: kevinbarabash

Reviewers: jeresig, kevinbarabash

Required Reviewers:

Approved By: jeresig

Checks: ✅ CodeQL, ⌛ Lint, typecheck, and coverage check (ubuntu-latest, 16.x), ✅ gerald, ✅ Prime node_modules cache for primary configuration (ubuntu-latest, 16.x), ✅ Analyze (javascript), ⏭  dependabot

Pull Request URL: #531

.changeset/soft-buses-unite.md

@@ -0,0 +1,5 @@
+---
+"@khanacademy/eslint-config": minor
+---
+
+Update eslint-khan-config and .eslintrc.js to lint .ts files

.eslintrc.js

@@ -1,12 +1,6 @@
 /* eslint-disable import/no-commonjs */
 module.exports = {
     extends: ["./packages/eslint-config-khan/index.js"],
-    parser: "@babel/eslint-parser",
-    parserOptions: {
-        babelOptions: {
-            configFile: "./build-settings/babel.config.js",
-        },
-    },
     plugins: ["@babel", "import", "jest", "promise", "monorepo", "disable"],
     settings: {
         "eslint-plugin-disable": {
@@ -15,6 +9,24 @@ module.exports = {
                 "jsx-a11y": ["./*.js", "src/*.js"],
             },
         },
+        "import/parsers": {
+            "@typescript-eslint/parser": [".ts", ".tsx"],
+        },
+        "import/resolver": {
+            typescript: {
+                project: [
+                    "packages/*/tsconfig.json",
+                    "packages/tsconfig-shared.json",
+                ],
+            },
+            node: {
+                project: [
+                    "packages/*/tsconfig.json",
+                    "packages/tsconfig-shared.json",
+                ],
+            },
+        },
+        "import/extensions": [".js", ".jsx", ".ts", ".tsx"],
     },
     globals: {
         __IS_BROWSER__: "readonly",
@@ -27,17 +39,29 @@ module.exports = {
             },
         },
         {
-            files: ["**/__tests__/**/*.test.js"],
+            files: ["**/__tests__/**/*.test.ts"],
             rules: {
                 "max-lines": "off",
+                "@typescript-eslint/no-empty-function": "off",
             },
         },
         {
-            files: ["**/bin/**/*.js"],
+            files: ["**/bin/**/*.ts", "build-scripts/*.ts"],
             rules: {
                 "no-console": "off",
             },
         },
+        {
+            files: [
+                "**/__tests__/**/*.test.ts",
+                "utils/*.js",
+                "config/**",
+                "build-settings/**",
+            ],
+            rules: {
+                "@typescript-eslint/no-var-requires": "off",
+            },
+        },
     ],
     rules: {
         "new-cap": "off",
@@ -85,7 +109,10 @@ module.exports = {
         "promise/no-new-statics": "error",
         "promise/no-return-in-finally": "error",
         "monorepo/no-internal-import": "error",
-        "monorepo/no-relative-import": "error",
+        // NOTE: This rule reports false positives for cross-module imports using
+        // `@khanacademy/wonder-stuff-*`.  This is likely due to a bad interaction
+        // with the settings we're using for `import/resolver`.
+        // "monorepo/no-relative-import": "error",
         "import/no-restricted-paths": [
             "error",
             {
@@ -97,5 +124,7 @@ module.exports = {
                 ],
             },
         ],
+
+        "@typescript-eslint/no-explicit-any": "off",
     },
 };

.github/workflows/node-ci.yml

@@ -83,7 +83,7 @@ jobs:
       uses: Khan/actions@full-or-limited-v0
       with:
         full-trigger: ${{ steps.eslint-reset.outputs.filtered }}
-        full: yarn lint:ci packages
+        full: yarn lint:ci
         limited-trigger: ${{ steps.js-files.outputs.filtered }}
         limited: yarn lint:ci {}
 

.vscode/settings.json

@@ -1,6 +1,12 @@
 {
     "editor.renderWhitespace": "boundary",
     "eslint.codeActionsOnSave.mode": "all",
+    "eslint.validate": [
+        "javascript",
+        "javascriptreact",
+        "typescript",
+        "typescriptreact",
+    ],
     "files.trimTrailingWhitespace": true,
     "typescript.format.enable": false,
     "typescript.validate.enable": true,

build-scripts/gen-flow-types.ts

@@ -1,9 +1,11 @@
 import {execSync} from "child_process";
-import * as fg from "fast-glob";
 import * as path from "path";
+import * as fglob from "fast-glob";
 
 const rootDir = path.join(__dirname, "..");
-const files = fg.sync("packages/wonder-stuff-*/dist/**/*.d.ts", {cwd: rootDir});
+const files = fglob.sync("packages/wonder-stuff-*/dist/**/*.d.ts", {
+    cwd: rootDir,
+});
 
 for (const inFile of files) {
     const outFile = inFile.replace(".d.ts", ".js.flow");

build-scripts/remove-test-types-from-dist.ts

@@ -1,17 +1,17 @@
 import * as fs from "fs";
-import * as fg from "fast-glob";
 import * as path from "path";
+import * as fglob from "fast-glob";
 
 const rootDir = path.join(__dirname, "..");
-const files = fg.sync("packages/wonder-stuff-*/dist/**/__tests__/*.d.ts", {
+const files = fglob.sync("packages/wonder-stuff-*/dist/**/__tests__/*.d.ts", {
     cwd: rootDir,
 });
 
 for (const file of files) {
     fs.unlinkSync(path.join(rootDir, file));
 }
 
-const dirs = fg.sync("packages/wonder-stuff-*/dist/**/__tests__", {
+const dirs = fglob.sync("packages/wonder-stuff-*/dist/**/__tests__", {
     cwd: rootDir,
     onlyFiles: false,
 });

package.json

@@ -34,9 +34,12 @@
     "@types/jest": "^29.4.0",
     "@types/jest-when": "^3.5.2",
     "@types/winston": "^2.4.4",
+    "@typescript-eslint/eslint-plugin": "^5.52.0",
+    "@typescript-eslint/parser": "^5.52.0",
     "babel-jest": "29.4.2",
     "eslint": "^8.34.0",
     "eslint-config-prettier": "^8.6.0",
+    "eslint-import-resolver-typescript": "^3.5.3",
     "eslint-plugin-disable": "^2.0.3",
     "eslint-plugin-eslint-comments": "^3.2.0",
     "eslint-plugin-import": "^2.27.5",
@@ -76,8 +79,8 @@
     "clean": "rm -rf packages/wonder-stuff-*/dist && rm -rf packages/wonder-stuff-*/node_modules && rm -f packages/*/tsconfig.tsbuildinfo",
     "coverage": "yarn run jest --coverage",
     "format": "prettier --write .",
-    "lint": "eslint --config .eslintrc.js packages",
-    "lint:ci": "eslint --config .eslintrc.js",
+    "lint": "yarn lint:ci .",
+    "lint:ci": "eslint --config .eslintrc.js --ext .ts --ext .js",
     "publish:ci": "node utils/pre-publish-check-ci.js && git diff --stat --exit-code HEAD && yarn build && yarn build:types && yarn build:flowtypes && changeset publish",
     "test": "yarn jest",
     "typecheck": "tsc --project tsconfig-check.json",

packages/eslint-config-khan/index.js

@@ -4,9 +4,14 @@ const WARN = "warn";
 const ERROR = "error";
 
 module.exports = {
-    parser: "babel-eslint",
-    plugins: ["jsx-a11y", "prettier", "react"],
-    extends: ["eslint:recommended", "prettier"],
+    parser: "@typescript-eslint/parser",
+    plugins: ["@typescript-eslint", "jsx-a11y", "prettier", "react"],
+    extends: [
+        "eslint:recommended",
+        "plugin:@typescript-eslint/eslint-recommended",
+        "prettier",
+        "plugin:@typescript-eslint/recommended",
+    ],
     env: {
         // TODO(csilvers): once we properly use node.js for node
         // files, get rid of this next line.
@@ -56,7 +61,9 @@ module.exports = {
         "no-unexpected-multiline": ERROR,
         "no-unreachable": ERROR,
         "no-unused-expressions": ERROR,
-        "no-unused-vars": [ERROR, {args: "none", varsIgnorePattern: "^_*$"}],
+        "no-unused-vars": OFF,
+        "@typescript-eslint/no-unused-vars": WARN,
+        //  {args: "none", varsIgnorePattern: "^_*$"}],
         "no-useless-call": ERROR,
         "no-var": ERROR,
         "no-with": ERROR,

packages/wonder-stuff-core/src/error-info.ts

@@ -179,8 +179,8 @@ export class ErrorInfo {
      */
     static normalize(
         error: Error,
-        stripFrames: number = 0,
-        minimumFrameCount: number = 1,
+        stripFrames = 0,
+        minimumFrameCount = 1,
     ): ErrorInfo {
         if (process.env.NODE_ENV !== "production") {
             // Verify our arguments.

packages/wonder-stuff-i18n/src/plugins/i18n-plugin.ts

@@ -143,6 +143,7 @@ export type Assets = {
     [assetName: string]: Asset;
 };
 
+// eslint-disable-next-line import/no-default-export
 export default class I18nPlugin {
     options: InternalOptions;
 

packages/wonder-stuff-i18n/src/utils/__tests__/pofile-utils.test.ts

@@ -1,7 +1,7 @@
-import fs from "fs";
-import path from "path";
+import * as fs from "fs";
+import * as path from "path";
 
-import fglob from "fast-glob";
+import * as fglob from "fast-glob";
 
 import {
     buildPoItem,
@@ -12,6 +12,22 @@ import {
 } from "../pofile-utils";
 import * as I18nUtils from "../i18n-utils";
 
+jest.mock("fs", () => {
+    const original = jest.requireActual("fs");
+    return {
+        __esModule: true,
+        ...original,
+    };
+});
+
+jest.mock("fast-glob", () => {
+    const original = jest.requireActual("fast-glob");
+    return {
+        __esModule: true,
+        ...original,
+    };
+});
+
 describe("buildPoItem", () => {
     it("builds a PO item from an extracted string", () => {
         // Arrange
@@ -361,6 +377,7 @@ i18n._("A singular string.");
 i18n.ngettext("%(num)s singular string.", "%(num)s plural string.", num);`;
 
         jest.spyOn(fs, "readFileSync")
+            .mockImplementation()
             .mockReturnValueOnce(file1)
             .mockReturnValueOnce(file2);
 

packages/wonder-stuff-i18n/src/utils/i18n-utils.ts

@@ -22,9 +22,7 @@ export type TranslatedLocaleStrings = {
  * @param {string} ignoreFile The file to read the glob strings from.
  * @returns {Array<string>} a list of glob strings to ignore
  */
-export const getIgnoreGlobs = (
-    ignoreFile: string = ".i18nignore",
-): Array<string> => {
+export const getIgnoreGlobs = (ignoreFile = ".i18nignore"): Array<string> => {
     try {
         // Find the ignore file, if it exists
         // If it doesn't exist then ancesdir throws an exception.

packages/wonder-stuff-i18n/src/utils/pofile-utils.ts

@@ -1,7 +1,7 @@
-import fs from "fs";
-import path from "path";
+import * as fs from "fs";
+import * as path from "path";
 
-import fglob from "fast-glob";
+import * as fglob from "fast-glob";
 import PO from "pofile";
 
 import {extractStrings} from "./extract-i18n";

packages/wonder-stuff-sentry/src/kind-error-data.ts

@@ -20,7 +20,7 @@ type InvalidTags = {
 };
 
 export class KindErrorData implements SentryIntegration {
-    static id: string = "KindErrorData";
+    static id = "KindErrorData";
     name: string = KindErrorData.id;
     readonly _options: KindErrorDataOptions;
 

packages/wonder-stuff-sentry/src/types.ts

@@ -1,8 +1,7 @@
 import type {Metadata} from "@khanacademy/wonder-stuff-core";
 
-namespace Flow {
-    export type Class<T> = new (...args: any[]) => T;
-}
+// Copied from https://github.com/Khan/flow-to-typescript-codemod/blob/main/flow.d.ts
+type Class<T> = new (...args: any[]) => T;
 
 /**
  * Tags for a Sentry event.
@@ -157,7 +156,7 @@ export type SentryEventProcessor = (
 
 export interface SentryHub {
     getIntegration<T extends SentryIntegration>(
-        integration: Flow.Class<T>,
+        integration: Class<T>,
     ): T | null | undefined;
 }
 

packages/wonder-stuff-server-google/src/add-app-engine-middleware.ts

@@ -1,5 +1,5 @@
 import express from "express";
-import type {Application, Request, Response} from "express";
+import type {Application} from "express";
 import {Runtime} from "@khanacademy/wonder-stuff-server";
 import type {Logger} from "@khanacademy/wonder-stuff-server";
 import * as lw from "@google-cloud/logging-winston";
@@ -8,10 +8,7 @@ import {makeAppEngineRequestIDMiddleware} from "./middleware/make-app-engine-req
 /**
  * Apply the middleware that we want to use with Google App Engine (GAE).
  */
-export async function addAppEngineMiddleware<
-    TReq extends Request,
-    TRes extends Response,
->(
+export async function addAppEngineMiddleware(
     app: Application,
     mode: (typeof Runtime)[keyof typeof Runtime],
     logger: Logger,

packages/wonder-stuff-server-google/src/start-server.ts

@@ -1,11 +1,10 @@
 import * as http from "http";
-import type {Application, Request, Response} from "express";
+import type {Application} from "express";
 import {
     startServer as startServerCore,
     Runtime,
     createLogger,
 } from "@khanacademy/wonder-stuff-server";
-import type {RequestWithLog} from "@khanacademy/wonder-stuff-server";
 import * as lw from "@google-cloud/logging-winston";
 import {addAppEngineMiddleware} from "./add-app-engine-middleware";
 import {setupIntegrations} from "./setup-integrations";
@@ -19,10 +18,7 @@ import {getDefaultLogMetadata} from "./get-default-log-metadata";
  * This takes a server application definition and attaches Goole Cloud
  * middleware before listening on the appropriate port per the passed options.
  */
-export async function startServer<
-    TReq extends RequestWithLog<Request>,
-    TRes extends Response,
->(
+export async function startServer(
     options: ServerOptions,
     app: Application,
 ): Promise<http.Server | null | undefined> {
@@ -56,7 +52,7 @@ export async function startServer<
     await setupIntegrations(restOptions.mode, integrations);
 
     // Add GAE middleware to the app.
-    const appWithMiddleware = await addAppEngineMiddleware<TReq, TRes>(
+    const appWithMiddleware = await addAppEngineMiddleware(
         app,
         restOptions.mode,
         logger,