From a50e40f309144e3e0f37594784f7ecc1fb27fff7 Mon Sep 17 00:00:00 2001 From: Elliot Winkler Date: Thu, 23 Jul 2026 10:18:12 -0600 Subject: [PATCH] Add lint:tsc package script Currently, the monorepo is not being fully typechecked in CI. This is a problem because type errors have sneaked into test files, and the only way to see them is in an editor context. To fix this, this commit adds another category of tsconfig files (`tsconfig.lint.json`) which are like the development-only versions, except they emit type declarations to a temporary directory. This allows packages which have dependencies on other packages to resolve types correctly. These tsconfigs are linted by the `lint:tsconfigs` scripts. This commit also adds a `lint:tsc` package script which makes use of `tsconfig.lint.json`. Note that if we were to run this across the monorepo we would have to address many type errors. To allow us to address these incrementally, this commit only enables linting for a subset of packages. --- .github/workflows/lint-build-test.yml | 1 + .gitignore | 1 + knip.config.ts => knip.config.mts | 0 package.json | 7 +- packages/base-controller/tsconfig.lint.json | 12 + packages/messenger-cli/tsconfig.lint.json | 8 + packages/messenger/tsconfig.lint.json | 7 + packages/platform-api-docs/tsconfig.lint.json | 8 + scripts/create-package/cli.test.ts | 2 +- .../lint-tsconfigs/lint-package-tsconfigs.mts | 208 +++++++++++++++--- .../lint-tsconfigs/lint-root-tsconfigs.mts | 126 +++++++++-- scripts/lint-tsconfigs/lint-tsconfigs.mts | 4 +- scripts/lint-tsconfigs/utils.mts | 35 ++- tsconfig.json | 2 +- tsconfig.lint.json | 24 ++ tsconfig.packages.lint.json | 9 + 16 files changed, 392 insertions(+), 62 deletions(-) rename knip.config.ts => knip.config.mts (100%) create mode 100644 packages/base-controller/tsconfig.lint.json create mode 100644 packages/messenger-cli/tsconfig.lint.json create mode 100644 packages/messenger/tsconfig.lint.json create mode 100644 packages/platform-api-docs/tsconfig.lint.json create mode 100644 tsconfig.lint.json create mode 100644 tsconfig.packages.lint.json diff --git a/.github/workflows/lint-build-test.yml b/.github/workflows/lint-build-test.yml index 76ff47fe639..5af0be17b0a 100644 --- a/.github/workflows/lint-build-test.yml +++ b/.github/workflows/lint-build-test.yml @@ -42,6 +42,7 @@ jobs: - lint:misc:check - lint:teams - lint:tsconfigs:all + - lint:tsc - messenger-action-types:check - readme-content:check steps: diff --git a/.gitignore b/.gitignore index 650326318de..3dbfdb48a7c 100644 --- a/.gitignore +++ b/.gitignore @@ -34,6 +34,7 @@ scripts/coverage !.yarn/versions # typescript +**/.tsc-lint-cache/ packages/*/*.tsbuildinfo # AI diff --git a/knip.config.ts b/knip.config.mts similarity index 100% rename from knip.config.ts rename to knip.config.mts diff --git a/package.json b/package.json index 0ce1658b60b..57c42f5a186 100644 --- a/package.json +++ b/package.json @@ -27,14 +27,15 @@ "docs:platform-api:build": "yarn workspace @metamask/platform-api-docs cli ../.. --build --project-label Core", "docs:platform-api:dev": "yarn workspace @metamask/platform-api-docs cli ../.. --dev --project-label Core", "docs:platform-api:serve": "yarn workspace @metamask/platform-api-docs cli ../.. --serve --project-label Core", - "lint": "yarn lint:eslint && echo && yarn lint:misc --check && yarn constraints && yarn lint:dependencies && yarn lint:teams && yarn messenger-action-types:check && yarn readme-content:check && yarn lint:tsconfigs:all && yarn codeowners:check", - "lint:dependencies": "knip --dependencies && yarn dedupe --check", - "lint:dependencies:fix": "knip --dependencies && yarn dedupe", + "lint": "yarn lint:tsc && yarn lint:eslint && echo && yarn lint:misc --check && yarn constraints && yarn lint:dependencies && yarn lint:teams && yarn messenger-action-types:check && yarn readme-content:check && yarn lint:tsconfigs:all && yarn codeowners:check", + "lint:dependencies": "knip --config knip.config.mts --dependencies && yarn dedupe --check", + "lint:dependencies:fix": "knip --config knip.config.mts --dependencies && yarn dedupe", "lint:eslint": "yarn build:only-clean && NODE_OPTIONS='--max-old-space-size=8192' yarn eslint", "lint:fix": "yarn lint:eslint --fix --prune-suppressions && echo && yarn lint:misc --write && yarn constraints --fix && yarn lint:dependencies:fix && yarn messenger-action-types:generate && yarn readme-content:update && yarn codeowners:generate", "lint:misc": "oxfmt --ignore-path .gitignore", "lint:misc:check": "yarn lint:misc --check", "lint:teams": "tsx scripts/lint-teams-json.ts", + "lint:tsc": "tsc --build tsconfig.lint.json", "lint:tsconfigs": "tsx scripts/lint-tsconfigs/lint-tsconfigs.mts", "lint:tsconfigs:all": "yarn workspaces foreach --all --parallel --interlaced --verbose run lint:tsconfigs", "lint:tsconfigs:fix": "tsx scripts/lint-tsconfigs/lint-tsconfigs.mts --fix", diff --git a/packages/base-controller/tsconfig.lint.json b/packages/base-controller/tsconfig.lint.json new file mode 100644 index 00000000000..1e03d050875 --- /dev/null +++ b/packages/base-controller/tsconfig.lint.json @@ -0,0 +1,12 @@ +{ + "extends": ["./tsconfig.json", "../../tsconfig.packages.lint.json"], + "compilerOptions": { + "outDir": "./.tsc-lint-cache", + "tsBuildInfoFile": "./.tsc-lint-cache/tsconfig.tsbuildinfo" + }, + "references": [ + { + "path": "../messenger/tsconfig.lint.json" + } + ] +} diff --git a/packages/messenger-cli/tsconfig.lint.json b/packages/messenger-cli/tsconfig.lint.json new file mode 100644 index 00000000000..fb65dcfce34 --- /dev/null +++ b/packages/messenger-cli/tsconfig.lint.json @@ -0,0 +1,8 @@ +{ + "extends": ["./tsconfig.json", "../../tsconfig.packages.lint.json"], + "compilerOptions": { + "outDir": "./.tsc-lint-cache", + "tsBuildInfoFile": "./.tsc-lint-cache/tsconfig.tsbuildinfo" + }, + "references": [] +} diff --git a/packages/messenger/tsconfig.lint.json b/packages/messenger/tsconfig.lint.json new file mode 100644 index 00000000000..378ed1dce55 --- /dev/null +++ b/packages/messenger/tsconfig.lint.json @@ -0,0 +1,7 @@ +{ + "extends": ["./tsconfig.json", "../../tsconfig.packages.lint.json"], + "compilerOptions": { + "outDir": "./.tsc-lint-cache", + "tsBuildInfoFile": "./.tsc-lint-cache/tsconfig.tsbuildinfo" + } +} diff --git a/packages/platform-api-docs/tsconfig.lint.json b/packages/platform-api-docs/tsconfig.lint.json new file mode 100644 index 00000000000..fb65dcfce34 --- /dev/null +++ b/packages/platform-api-docs/tsconfig.lint.json @@ -0,0 +1,8 @@ +{ + "extends": ["./tsconfig.json", "../../tsconfig.packages.lint.json"], + "compilerOptions": { + "outDir": "./.tsc-lint-cache", + "tsBuildInfoFile": "./.tsc-lint-cache/tsconfig.tsbuildinfo" + }, + "references": [] +} diff --git a/scripts/create-package/cli.test.ts b/scripts/create-package/cli.test.ts index 330f9838ded..8ddecfb8653 100644 --- a/scripts/create-package/cli.test.ts +++ b/scripts/create-package/cli.test.ts @@ -44,7 +44,7 @@ describe('create-package/cli', () => { beforeEach(() => { // yargs calls process.exit() with 1 on failure and sometimes 0 on success. // We have to intercept it. - jest.spyOn(process, 'exit').mockImplementation((code?: number) => { + jest.spyOn(process, 'exit').mockImplementation((code) => { if (code === 1) { throw new Error('exit: 1'); } else { diff --git a/scripts/lint-tsconfigs/lint-package-tsconfigs.mts b/scripts/lint-tsconfigs/lint-package-tsconfigs.mts index e3d77708c4d..e50e79f43f7 100644 --- a/scripts/lint-tsconfigs/lint-package-tsconfigs.mts +++ b/scripts/lint-tsconfigs/lint-package-tsconfigs.mts @@ -1,10 +1,14 @@ +import * as path from 'path'; + import type { PackageManifest, TsconfigLintMetaReport, + Workspace, Workspaces, } from './utils.mjs'; import { ensureTsconfigsUpdated, + filterWorkspacesWithTsconfig, getAllNonRootWorkspaces, lintTsconfigs, printReport, @@ -13,8 +17,9 @@ import { } from './utils.mjs'; /** - * Lints a package's tsconfig.json and tsconfig.build.json files to ensure they - * reference all workspace dependencies. Optionally fixes any issues found. + * Lints a package's development, build, and lint TypeScript configs to ensure + * they reference the expected workspace dependencies. Optionally fixes any + * issues found. * * @param options - The options object. * @param options.packageRoot - The root directory of the package. @@ -33,45 +38,158 @@ export async function lintPackageTsconfigs({ }): Promise { const manifest = await readPackageManifest(packageRoot); const workspaces = await getAllNonRootWorkspaces(repoRoot); - const tsconfigs = await Promise.all([ + const expectedPackageNames = getExpectedWorkspaceDependencies({ + manifest, + workspaces, + }); + + const devAndBuildReport = await lintPackageDevAndBuildOnlyTsconfigs({ + packageRoot, + repoRoot, + workspaces, + expectedPackageNames, + shouldFix, + }); + const lintReport = await lintPackageLintOnlyTsconfigs({ + packageRoot, + repoRoot, + manifest, + workspaces, + expectedPackageNames, + shouldFix, + }); + + return devAndBuildReport.didPass && lintReport.didPass; +} + +/** + * Lints a package's development- and build-only TypeScript configs to ensure + * they reference the expected workspace dependencies. Optionally fixes any + * issues found. + * + * @param options - The options object. + * @param options.packageRoot - The root directory of the package. + * @param options.repoRoot - The root directory of the repository. + * @param options.workspaces - The workspaces in the repository. + * @param options.expectedPackageNames - Workspace dependencies to reference. + * @param options.shouldFix - Whether to automatically fix issues. + * @returns The report resulting from the lint. + */ +export async function lintPackageDevAndBuildOnlyTsconfigs({ + packageRoot, + repoRoot, + workspaces, + expectedPackageNames, + shouldFix, +}: { + packageRoot: string; + repoRoot: string; + workspaces: Workspaces; + expectedPackageNames: Set; + shouldFix: boolean; +}): Promise { + const devAndBuildTsconfigs = await Promise.all([ readTsconfig(packageRoot, 'tsconfig.json'), readTsconfig(packageRoot, 'tsconfig.build.json'), ]); - const expectedPackageNames = getExpectedWorkspaceDependencies({ - manifest, + const expectedWorkspaces = getSortedWorkspaces({ + packageNames: expectedPackageNames, workspaces, }); - const sortedExpectedWorkspaces = [...expectedPackageNames] - .sort((a, b) => a.localeCompare(b)) - .map((name) => { - const workspace = workspaces.byName.get(name); - if (!workspace) { - throw new Error(`Expected workspace not found for package: ${name}`); - } - return workspace; - }); - let report: TsconfigLintMetaReport; - if (shouldFix) { - report = await ensureTsconfigsUpdated({ - workspaces: sortedExpectedWorkspaces, - tsconfigs, - repoRoot, - currentWorkspaceRoot: packageRoot, - }); - } else { - report = await lintTsconfigs({ - tsconfigs, - expectedPackageNames, - workspaces, - repoRoot, - currentWorkspaceRoot: packageRoot, - }); - } + const report = shouldFix + ? await ensureTsconfigsUpdated({ + workspaces: expectedWorkspaces, + tsconfigs: devAndBuildTsconfigs, + repoRoot, + currentWorkspaceRoot: packageRoot, + }) + : await lintTsconfigs({ + tsconfigs: devAndBuildTsconfigs, + expectedPackageNames, + workspaces, + repoRoot, + currentWorkspaceRoot: packageRoot, + }); + + printReport(report); + + return report; +} + +/** + * Lints a package's lint-only TypeScript config to ensure it references the + * expected workspace dependencies. Optionally fixes any issues found. + * + * @param options - The options object. + * @param options.packageRoot - The root directory of the package. + * @param options.repoRoot - The root directory of the repository. + * @param options.manifest - Contents of the package's `package.json` file. + * @param options.workspaces - The workspaces in the repository. + * @param options.expectedPackageNames - Workspace dependencies to reference. + * @param options.shouldFix - Whether to automatically fix issues. + * @returns The report resulting from the lint. + */ +export async function lintPackageLintOnlyTsconfigs({ + packageRoot, + repoRoot, + manifest, + workspaces, + expectedPackageNames, + shouldFix, +}: { + packageRoot: string; + repoRoot: string; + manifest: PackageManifest; + workspaces: Workspaces; + expectedPackageNames: Set; + shouldFix: boolean; +}): Promise { + const expectedWorkspaces = getSortedWorkspaces({ + packageNames: expectedPackageNames, + workspaces, + }); + const lintWorkspaces = await filterWorkspacesWithTsconfig({ + workspaces: expectedWorkspaces, + repoRoot, + fileName: 'tsconfig.lint.json', + }); + // This allows us to increase linting for the whole repo incrementally. + const packageLintWorkspaces = await filterWorkspacesWithTsconfig({ + workspaces: [ + { + name: manifest.name, + location: path.relative(repoRoot, packageRoot), + }, + ], + repoRoot, + fileName: 'tsconfig.lint.json', + }); + const lintTsconfigsToCheck = + packageLintWorkspaces.length === 0 + ? [] + : [await readTsconfig(packageRoot, 'tsconfig.lint.json')]; + + const report = shouldFix + ? await ensureTsconfigsUpdated({ + workspaces: lintWorkspaces, + tsconfigs: lintTsconfigsToCheck, + repoRoot, + currentWorkspaceRoot: packageRoot, + }) + : await lintTsconfigs({ + tsconfigs: lintTsconfigsToCheck, + expectedPackageNames: new Set( + lintWorkspaces.map((workspace) => workspace.name), + ), + workspaces, + repoRoot, + currentWorkspaceRoot: packageRoot, + }); printReport(report); - return report.didPass; + return report; } /** @@ -101,3 +219,29 @@ function getExpectedWorkspaceDependencies({ ), ); } + +/** + * Gets workspace metadata for the given package names in alphabetical order. + * + * @param options - The options object. + * @param options.packageNames - Workspace package names to resolve. + * @param options.workspaces - The workspaces in the repository. + * @returns The corresponding workspaces in alphabetical order. + */ +function getSortedWorkspaces({ + packageNames, + workspaces, +}: { + packageNames: Set; + workspaces: Workspaces; +}): Workspace[] { + return [...packageNames] + .sort((a, b) => a.localeCompare(b)) + .map((name) => { + const workspace = workspaces.byName.get(name); + if (!workspace) { + throw new Error(`Expected workspace not found for package: ${name}`); + } + return workspace; + }); +} diff --git a/scripts/lint-tsconfigs/lint-root-tsconfigs.mts b/scripts/lint-tsconfigs/lint-root-tsconfigs.mts index b0ba2016190..d9858324b49 100644 --- a/scripts/lint-tsconfigs/lint-root-tsconfigs.mts +++ b/scripts/lint-tsconfigs/lint-root-tsconfigs.mts @@ -1,6 +1,7 @@ -import type { TsconfigLintMetaReport } from './utils.mjs'; +import type { TsconfigLintMetaReport, Workspaces } from './utils.mjs'; import { ensureTsconfigsUpdated, + filterWorkspacesWithTsconfig, getAllNonRootWorkspaces, lintTsconfigs, printReport, @@ -8,8 +9,9 @@ import { } from './utils.mjs'; /** - * Lints the root tsconfig.json and tsconfig.build.json files to ensure they - * reference all workspace packages. Optionally fixes any issues found. + * Lints the root development, build, and lint TypeScript configs to ensure they + * reference the expected workspace packages. Optionally fixes any issues + * found. * * @param options - The options object. * @param options.repoRoot - The root directory of the repository. @@ -24,30 +26,110 @@ export async function lintRootTsconfigs({ shouldFix: boolean; }): Promise { const workspaces = await getAllNonRootWorkspaces(repoRoot); - const tsconfigs = await Promise.all([ + + const devAndBuildReport = await lintRootDevAndBuildOnlyTsconfigs({ + repoRoot, + workspaces, + shouldFix, + }); + const lintReport = await lintRootLintOnlyTsconfigs({ + repoRoot, + workspaces, + shouldFix, + }); + + return devAndBuildReport.didPass && lintReport.didPass; +} + +/** + * Lints the root development- and build-only TypeScript configs to ensure they + * reference the expected workspace packages. Optionally fixes any issues found. + * + * @param options - The options object. + * @param options.repoRoot - The root directory of the repository. + * @param options.workspaces - The workspaces in the repository. + * @param options.shouldFix - Whether to automatically fix issues. + * @returns The report resulting from the lint. + */ +export async function lintRootDevAndBuildOnlyTsconfigs({ + repoRoot, + workspaces, + shouldFix, +}: { + repoRoot: string; + workspaces: Workspaces; + shouldFix: boolean; +}): Promise { + const devAndBuildTsconfigs = await Promise.all([ readTsconfig(repoRoot, 'tsconfig.json'), readTsconfig(repoRoot, 'tsconfig.build.json'), ]); - let report: TsconfigLintMetaReport; - if (shouldFix) { - report = await ensureTsconfigsUpdated({ - workspaces: workspaces.list, - tsconfigs, - repoRoot, - currentWorkspaceRoot: repoRoot, - }); - } else { - report = await lintTsconfigs({ - tsconfigs, - expectedPackageNames: workspaces.names, - workspaces, - repoRoot, - currentWorkspaceRoot: repoRoot, - }); - } + const report = shouldFix + ? await ensureTsconfigsUpdated({ + workspaces: workspaces.list, + tsconfigs: devAndBuildTsconfigs, + repoRoot, + currentWorkspaceRoot: repoRoot, + }) + : await lintTsconfigs({ + tsconfigs: devAndBuildTsconfigs, + expectedPackageNames: workspaces.names, + workspaces, + repoRoot, + currentWorkspaceRoot: repoRoot, + }); + + printReport(report); + + return report; +} + +/** + * Lints the root lint-only TypeScript configs to ensure they + * reference the expected workspace packages. Optionally fixes any issues found. + * + * @param options - The options object. + * @param options.repoRoot - The root directory of the repository. + * @param options.workspaces - The workspaces in the repository. + * @param options.shouldFix - Whether to automatically fix issues. + * @returns The report resulting from the lint. + */ +export async function lintRootLintOnlyTsconfigs({ + repoRoot, + workspaces, + shouldFix, +}: { + repoRoot: string; + workspaces: Workspaces; + shouldFix: boolean; +}): Promise { + const lintTsconfig = await readTsconfig(repoRoot, 'tsconfig.lint.json'); + // This allows us to increase linting for the whole repo incrementally. + const lintWorkspaces = await filterWorkspacesWithTsconfig({ + workspaces: workspaces.list, + repoRoot, + fileName: 'tsconfig.lint.json', + }); + + const report = shouldFix + ? await ensureTsconfigsUpdated({ + workspaces: lintWorkspaces, + tsconfigs: [lintTsconfig], + repoRoot, + currentWorkspaceRoot: repoRoot, + }) + : await lintTsconfigs({ + tsconfigs: [lintTsconfig], + expectedPackageNames: new Set( + lintWorkspaces.map((workspace) => workspace.name), + ), + workspaces, + repoRoot, + currentWorkspaceRoot: repoRoot, + }); printReport(report); - return report.didPass; + return report; } diff --git a/scripts/lint-tsconfigs/lint-tsconfigs.mts b/scripts/lint-tsconfigs/lint-tsconfigs.mts index 2c9d017ba0e..127d50d1db0 100644 --- a/scripts/lint-tsconfigs/lint-tsconfigs.mts +++ b/scripts/lint-tsconfigs/lint-tsconfigs.mts @@ -13,8 +13,8 @@ await main(); * Detects whether the script is being run from the repository root or from * within a package, then lints the appropriate tsconfig files. * - * When run from the root, lints only the root tsconfig pair. - * When run from a package, lints only that package's tsconfig pair. + * When run from the root, lints only the root TypeScript configs. + * When run from a package, lints only that package's TypeScript configs. */ async function main(): Promise { const cwd = process.cwd(); diff --git a/scripts/lint-tsconfigs/utils.mts b/scripts/lint-tsconfigs/utils.mts index 731c3da3e8c..8dfe2bed8c5 100644 --- a/scripts/lint-tsconfigs/utils.mts +++ b/scripts/lint-tsconfigs/utils.mts @@ -1,4 +1,4 @@ -import { readJsonFile, readFile } from '@metamask/utils/node'; +import { readJsonFile, readFile, fileExists } from '@metamask/utils/node'; import * as commentJson from 'comment-json'; import execa from 'execa'; import fs from 'fs'; @@ -116,6 +116,39 @@ export async function getAllNonRootWorkspaces( }; } +/** + * Filters workspaces to those that contain a particular TypeScript config. + * + * @param args - The arguments to this function. + * @param args.workspaces - The workspaces to filter. + * @param args.repoRoot - The root directory of the repository. + * @param args.fileName - The TypeScript config filename to look for. + * @returns The workspaces containing the requested TypeScript config. + */ +export async function filterWorkspacesWithTsconfig({ + workspaces, + repoRoot, + fileName, +}: { + workspaces: readonly Workspace[]; + repoRoot: string; + fileName: string; +}): Promise { + const results = await Promise.all( + workspaces.map(async (workspace) => { + const filePath = path.resolve(repoRoot, workspace.location, fileName); + if (await fileExists(filePath)) { + return workspace; + } + return undefined; + }), + ); + + return results.filter( + (workspace): workspace is Workspace => workspace !== undefined, + ); +} + /** * Reads and parses a package manifest from a package root directory. * diff --git a/tsconfig.json b/tsconfig.json index 83755477993..e7d0eba8840 100644 --- a/tsconfig.json +++ b/tsconfig.json @@ -309,6 +309,6 @@ "./types", "./scripts", "./codeowners.ts", - "./knip.config.ts" + "./knip.config.mts" ] } diff --git a/tsconfig.lint.json b/tsconfig.lint.json new file mode 100644 index 00000000000..d04e36333ff --- /dev/null +++ b/tsconfig.lint.json @@ -0,0 +1,24 @@ +{ + /** + * This configuration incrementally enables repository-wide type checking. + */ + "extends": "./tsconfig.json", + "compilerOptions": { + "skipLibCheck": true, + "tsBuildInfoFile": "./.tsc-lint-cache/lint.tsbuildinfo" + }, + "references": [ + { + "path": "./packages/base-controller/tsconfig.lint.json" + }, + { + "path": "./packages/messenger/tsconfig.lint.json" + }, + { + "path": "./packages/messenger-cli/tsconfig.lint.json" + }, + { + "path": "./packages/platform-api-docs/tsconfig.lint.json" + } + ] +} diff --git a/tsconfig.packages.lint.json b/tsconfig.packages.lint.json new file mode 100644 index 00000000000..9d6e40495bc --- /dev/null +++ b/tsconfig.packages.lint.json @@ -0,0 +1,9 @@ +{ + /** + * This configuration is extended by each package's `tsconfig.lint.json` configuration. + */ + "compilerOptions": { + "emitDeclarationOnly": true, + "skipLibCheck": true + } +}