FEI-4957.6: Use project references and multiple tsconfig.json files (#…

…528) ## Summary: I ran into a number of issues with dts-bundle-generator. It exports private that weren't exported and it can't handle having multiple types with the same name, e.g. different 'Options' types are defined in multiple places in wonder-stuff-server. This PR address the issue by using project references which allow us to have multiple tsconfig.json files (one for each package) in a single repo. Each tsconfig.json defines where the root (source) and output directories are relative to the tsconfig.json file. The root tsconfig.json doesn't cover any source files and instead only references the other tsconfig.json in the package folders. https://www.typescriptlang.org/docs/handbook/project-references.html#composite Issue: FEI-4957 TODO: - [x] update the existing build scripts to not generate a .d.ts file or .flow.js - [x] update CI to run build:types ## Test plan: - remove the `@ts-expect-error` comment from isolate-modules.ts, see that there's an error reported in VSCode - `yarn clean` - `yarn build:types`, see the same error reported - `yarn typecheck`, see the same error - re-add the the `@ts-expect-error` comment, see the error is gone in VSCode - re-run the `yarn` commands from above, see that the error is no longer reported by them Author: kevinbarabash Reviewers: jeresig, kevinbarabash Required Reviewers: Approved By: jeresig, jeresig Checks: ✅ CodeQL, ⌛ Lint, typecheck, and coverage check (ubuntu-latest, 16.x), ⏭ dependabot, ✅ gerald, ✅ Prime node_modules cache for primary configuration (ubuntu-latest, 16.x), ✅ Analyze (javascript) Pull Request URL: #528
Khan · Feb 16, 2023 · da34455 · da34455
1 parent f37b289
commit da34455
Show file tree

Hide file tree

Showing 21 changed files with 172 additions and 213 deletions.
diff --git a/.changeset/nine-ravens-bake.md b/.changeset/nine-ravens-bake.md
@@ -0,0 +1,5 @@
+---
+"ws-dev-build-settings": patch
+---
+
+Don't generate index.js.flow and index.d.ts files
diff --git a/.github/workflows/node-ci.yml b/.github/workflows/node-ci.yml
@@ -72,6 +72,13 @@ jobs:
     - name: Build so that inter-package references are resolved
       run: yarn build
 
+    - name: Build Types
+      # There isn't an easy way for us to type check our tests without
+      # also including them in dist folder when we build the .d.ts files
+      # so we have to manually remove them.
+      # TODO(kevin): Figure out how to have the `build:types` script do this.
+      run: yarn build:types && rm -rf packages/*/dist/**/__tests__
+
     # Linting / type checking
     - name: Eslint
       uses: Khan/actions@full-or-limited-v0

diff --git a/.gitignore b/.gitignore
@@ -4,4 +4,5 @@ flow-coverage
 dist
 !.vscode
 .DS_Store
-yarn-error.log
+yarn-error.log
+tsconfig.tsbuildinfo
diff --git a/build-settings/rollup.config.js b/build-settings/rollup.config.js
@@ -4,7 +4,6 @@ import path from "path";
 import autoExternal from "rollup-plugin-auto-external";
 import {babel} from "@rollup/plugin-babel";
 import {terser} from "rollup-plugin-terser";
-import copy from "rollup-plugin-copy";
 import resolve from "@rollup/plugin-node-resolve";
 import replace from "@rollup/plugin-replace";
 import filesize from "rollup-plugin-filesize";
@@ -175,34 +174,6 @@ const getPackageInfo = (commandLineArgs, pkgName) => {
     const cjsBrowser = browser == null ? null : browser[cjsNode];
     const esmBrowser = browser == null ? null : browser[esmNode];
 
-    // This generates the flow import file and a file for intellisense to work.
-    // By using the same instance of it across all output configurations
-    // while using the `copyOnce` value, we ensure it only gets copied one time.
-    const typesAndDocsCopy = copy({
-        copyOnce: true,
-        verbose: true,
-        targets: [
-            // This is the file that provides flow types.
-            {
-                // src path is relative to the package root unless started
-                // with ./
-                src: "build-settings/index.js.flow.template",
-                // dest path is relative to src path.
-                dest: makePackageBasedPath(pkgName, "./dist"),
-                rename: "index.js.flow",
-            },
-            // This is the file that we use for intellisense.
-            {
-                // src path is relative to the package root unless started
-                // with ./
-                src: "build-settings/index.js.flow.template",
-                // dest path is relative to src path.
-                dest: makePackageBasedPath(pkgName, "./dist"),
-                rename: "index.d.ts",
-            },
-        ],
-    });
-
     const configs = [];
     if (platforms.has("browser")) {
         if (formats.has("cjs") && cjsBrowser) {
@@ -211,7 +182,7 @@ const getPackageInfo = (commandLineArgs, pkgName) => {
                 format: "cjs",
                 platform: "browser",
                 file: cjsBrowser,
-                plugins: [typesAndDocsCopy],
+                plugins: [],
             });
         }
         if (formats.has("esm") && esmBrowser) {
@@ -221,7 +192,7 @@ const getPackageInfo = (commandLineArgs, pkgName) => {
                 platform: "browser",
                 file: esmBrowser,
                 // We care about the file size of this one.
-                plugins: [typesAndDocsCopy, filesize()],
+                plugins: [filesize()],
             });
         }
     }
@@ -232,7 +203,7 @@ const getPackageInfo = (commandLineArgs, pkgName) => {
                 format: "cjs",
                 platform: "node",
                 file: cjsNode,
-                plugins: [typesAndDocsCopy],
+                plugins: [],
             });
         }
         if (formats.has("esm") && esmNode) {
@@ -241,7 +212,7 @@ const getPackageInfo = (commandLineArgs, pkgName) => {
                 format: "esm",
                 platform: "node",
                 file: esmNode,
-                plugins: [typesAndDocsCopy],
+                plugins: [],
             });
         }
     }

diff --git a/package.json b/package.json
@@ -60,7 +60,6 @@
     "prettier": "^2.8.4",
     "rollup": "^2.79.1",
     "rollup-plugin-auto-external": "^2.0.0",
-    "rollup-plugin-copy": "^3.4.0",
     "rollup-plugin-executable-output": "^1.3.0",
     "rollup-plugin-filesize": "^9.1.2",
     "rollup-plugin-preserve-shebangs": "^0.2.0",
@@ -74,15 +73,16 @@
   "scripts": {
     "build": "rollup -c build-settings/rollup.config.js",
     "build:prodsizecheck": "rollup -c build-settings/rollup.config.js --configPlatforms='browser' --configFormats='esm' --configEnvironment='production'",
+    "build:types": "yarn tsc --build --verbose tsconfig-build.json",
     "watch": "rollup -c build-settings/rollup.config.js --watch",
-    "clean": "rm -rf packages/wonder-stuff-*/dist && rm -rf packages/wonder-stuff-*/node_modules",
+    "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",
     "publish:ci": "node utils/pre-publish-check-ci.js && git diff --stat --exit-code HEAD && yarn build && changeset publish",
     "test": "yarn jest",
-    "typecheck": "tsc --noEmit",
+    "typecheck": "tsc --project tsconfig-check.json",
     "nochangeset": "yarn changeset add --empty"
   }
-}
+}
diff --git a/packages/tsconfig-shared.json b/packages/tsconfig-shared.json
@@ -0,0 +1,20 @@
+// This file is used by the tsconfig.json files in each package.
+/* Visit https://aka.ms/tsconfig to read more about this file */
+{
+  "extends": "../tsconfig-common.json",
+  "compilerOptions": {
+    "composite": true,
+    "incremental": true, // Required for composite projects
+
+    // We use rollup + babel to compile our bundles so we
+    // only need tsc to output .d.ts files
+    "declaration": true,
+    "emitDeclarationOnly": true,
+
+    "paths": {
+      "@khanacademy/*": [
+        "./*/src"
+      ]
+    },
+  },
+}
diff --git a/packages/wonder-stuff-core/tsconfig.json b/packages/wonder-stuff-core/tsconfig.json
@@ -0,0 +1,8 @@
+{
+  "exclude": ["dist", "**/*.flowtest.ts"],
+  "extends": "../tsconfig-shared.json",
+  "compilerOptions": {
+    "outDir": "dist",
+    "rootDir": "src",
+  }
+}
diff --git a/packages/wonder-stuff-i18n/tsconfig.json b/packages/wonder-stuff-i18n/tsconfig.json
@@ -0,0 +1,8 @@
+{
+  "exclude": ["dist", "**/*.flowtest.ts"],
+  "extends": "../tsconfig-shared.json",
+  "compilerOptions": {
+    "outDir": "dist",
+    "rootDir": "src",
+  }
+}
diff --git a/packages/wonder-stuff-sentry/src/kind-error-data.ts b/packages/wonder-stuff-sentry/src/kind-error-data.ts
@@ -72,7 +72,6 @@ export class KindErrorData implements SentryIntegration {
                 if (!self) {
                     return event;
                 }
-                // @ts-expect-error [FEI-5011] - TS2339 - Property 'enhanceEventWithErrorData' does not exist on type 'SentryIntegration'.
                 return self.enhanceEventWithErrorData(event, hint);
             },
         );

diff --git a/packages/wonder-stuff-sentry/src/types.ts b/packages/wonder-stuff-sentry/src/types.ts
@@ -1,7 +1,9 @@
-// @ts-expect-error [FEI-5011] - TS2307 - Cannot find module 'flow-to-typescript-codemod' or its corresponding type declarations.
-import {Flow} from "flow-to-typescript-codemod";
 import type {Metadata} from "@khanacademy/wonder-stuff-core";
 
+namespace Flow {
+    export type Class<T> = new (...args: any[]) => T;
+}
+
 /**
  * Tags for a Sentry event.
  *

diff --git a/packages/wonder-stuff-sentry/tsconfig.json b/packages/wonder-stuff-sentry/tsconfig.json
@@ -0,0 +1,11 @@
+{
+  "exclude": ["dist", "**/*.flowtest.ts"],
+  "extends": "../tsconfig-shared.json",
+  "compilerOptions": {
+    "outDir": "dist",
+    "rootDir": "src",
+  },
+  "references": [
+    {"path": "../wonder-stuff-core"}
+  ]
+}
diff --git a/packages/wonder-stuff-server-google/src/add-app-engine-middleware.ts b/packages/wonder-stuff-server-google/src/add-app-engine-middleware.ts
@@ -12,14 +12,11 @@ export async function addAppEngineMiddleware<
     TReq extends Request,
     TRes extends Response,
 >(
-    // @ts-expect-error [FEI-5011] - TS2315 - Type 'Application' is not generic.
-    app: Application<TReq, TRes>,
+    app: Application,
     mode: (typeof Runtime)[keyof typeof Runtime],
     logger: Logger,
-    // @ts-expect-error [FEI-5011] - TS2315 - Type 'Application' is not generic.
-): Promise<Application<TReq, TRes>> {
-    // @ts-expect-error [FEI-5011] - TS2558 - Expected 0 type arguments, but got 2.
-    const middlewareApp = express<TReq, TRes>();
+): Promise<Application> {
+    const middlewareApp = express();
 
     /**
      * If we're in production, we use the logging-winston middleware from

diff --git a/packages/wonder-stuff-server-google/tsconfig.json b/packages/wonder-stuff-server-google/tsconfig.json
@@ -0,0 +1,12 @@
+{
+  "exclude": ["dist", "**/*.flowtest.ts"],
+  "extends": "../tsconfig-shared.json",
+  "compilerOptions": {
+    "outDir": "dist",
+    "rootDir": "src",
+  },
+  "references": [
+    {"path": "../wonder-stuff-core"},
+    {"path": "../wonder-stuff-server"}
+  ]
+}
diff --git a/packages/wonder-stuff-server/src/start-server.ts b/packages/wonder-stuff-server/src/start-server.ts
@@ -34,8 +34,7 @@ export async function startServer<
         includeErrorMiddleware = true,
         includeRequestMiddleware = true,
     }: ServerOptions,
-    // @ts-expect-error [FEI-5011] - TS2315 - Type 'Application' is not generic.
-    app: Application<TReq, TRes>,
+    app: Application,
 ): Promise<http.Server | null | undefined> {
     /**
      * Setup logging.

diff --git a/packages/wonder-stuff-server/tsconfig.json b/packages/wonder-stuff-server/tsconfig.json
@@ -0,0 +1,11 @@
+{
+  "exclude": ["dist", "**/*.flowtest.ts"],
+  "extends": "../tsconfig-shared.json",
+  "compilerOptions": {
+    "outDir": "./dist",
+    "rootDir": "src",
+  },
+  "references": [
+    {"path": "../wonder-stuff-core"}
+  ]
+}
diff --git a/packages/wonder-stuff-testing/tsconfig.json b/packages/wonder-stuff-testing/tsconfig.json
@@ -0,0 +1,11 @@
+{
+  "exclude": ["dist", "**/*.flowtest.ts"],
+  "extends": "../tsconfig-shared.json",
+  "compilerOptions": {
+    "outDir": "dist",
+    "rootDir": "src",
+  },
+  "references": [
+    {"path": "../wonder-stuff-core"}
+  ]
+}
diff --git a/tsconfig-build.json b/tsconfig-build.json
@@ -0,0 +1,15 @@
+// This file is used by the `build:types` command in package.json
+/* Visit https://aka.ms/tsconfig to read more about this file */
+{
+  // Intentionally empty, all files are covered by tsconfig.json files
+  // in each package directory.
+  "files": [],
+  "references": [
+    {"path": "./packages/wonder-stuff-core"},
+    {"path": "./packages/wonder-stuff-i18n"},
+    {"path": "./packages/wonder-stuff-sentry"},
+    {"path": "./packages/wonder-stuff-server"},
+    {"path": "./packages/wonder-stuff-server-google"},
+    {"path": "./packages/wonder-stuff-testing"},
+  ],
+}
diff --git a/tsconfig-check.json b/tsconfig-check.json
@@ -0,0 +1,13 @@
+// This file is used by the `typecheck` command in package.json
+/* Visit https://aka.ms/tsconfig to read more about this file */
+{
+    "exclude": ["**/dist", "**/*.flowtest.ts"],
+    "extends": "./tsconfig-common.json",
+    "compilerOptions": {
+        "noEmit": true,
+
+        "paths": {
+            "@khanacademy/*": ["./packages/*/src"]
+        },
+    }
+}
diff --git a/tsconfig-common.json b/tsconfig-common.json
@@ -0,0 +1,30 @@
+// This file contains common compiler options that are used by all tsconfigs
+/* Visit https://aka.ms/tsconfig to read more about this file */
+{
+    "compilerOptions": {
+        /* Language and Environment */
+        "target": "ES2016",
+
+        /* Modules */
+        // Required for dynamic imports even though we aren't using
+        // tsc to output any modules.
+        "module": "ESNext",
+        "moduleResolution": "node",
+
+        /* Interop Constraints */
+        "esModuleInterop": true,
+        "forceConsistentCasingInFileNames": true,
+
+        /* Type Checking */
+        "strict": true,
+        "strictNullChecks": true,
+        "strictFunctionTypes": true,
+        "strictPropertyInitialization": true,
+        "strictBindCallApply": true,
+        "noImplicitAny": true,
+
+        /* Completeness */
+        "skipDefaultLibCheck": true, // it's safe to assume that built-in .d.ts files are correct
+        "skipLibCheck": false
+    }
+}