From 2b3b2ace7a3d76b2b90fd37cc627dc6752663105 Mon Sep 17 00:00:00 2001 From: Kris McGinnes Date: Wed, 14 Oct 2026 18:10:43 -0500 Subject: [PATCH 1/2] Type check with vp check (#2419) ## Summary Before `tsc` can go, we need to know whether Vite+'s type check catches everything it does. This turns on `lint.options.typeCheck`, so `vp check` and `vp lint` also report TypeScript errors through tsgolint. `pnpm checks` still runs `tsc` afterwards, so CI runs both side by side until [Retire tsc now that vp check type checks (#2420)](https://github.com/aws/graph-explorer/pull/2420) lands. ```diff options: { typeAware: true, + typeCheck: true, }, ``` `AGENTS.md` and `docs/development.md` no longer say `vp check` skips type checks. ## Evidence I planted each error below in a new or existing file, then ran `vp lint` and `pnpm check:types`. Both exit 1, with the same TypeScript code, for: - **All 3 packages:** a plain type error (TS2322), an implicit `any` (TS7006), an unused parameter and an unused local (TS6133, even with `no-unused-vars` disabled) - **graph-explorer:** `Object.groupBy` outside `lib: ES2023` (TS2550), a bad `@/` or `@shared/` path (TS2307), a `.tsx` prop type error, and a type error in a `.test.ts` file - **shared:** `Promise.withResolvers` outside `lib` (TS2550), a `.ts` import without `allowImportingTsExtensions` (TS5097), a missing `override` (TS4114), an `enum` under `erasableSyntaxOnly` (TS1294), and unreachable code - **proxy server:** an extensionless relative import under `nodenext` (TS2835) - **Config files:** a type error in the root and graph-explorer `vite.config.ts` `vp check` also catches a type error in `packages/shared/vite.config.ts` and `packages/graph-explorer-proxy-server/vite.config.ts`. `tsc` misses both, because no tsconfig includes those files. **Timing:** `vp check` takes 12.8s for the whole repo, with lint and types together in 8.7s of that. `pnpm check:types` takes 3.4s. The pre-commit hook on a single file takes 3 to 6s whether `typeCheck` is on or off. ## Merge Danger **Door:** two-way **Impact:** contributors Type errors now also fail the pre-commit hook for the files being committed. ## Related - Related to #2305 From df7153debbd4eaab5a31a69538596df509f5986c Mon Sep 17 00:00:00 2001 From: Kris McGinnes Date: Wed, 14 Oct 2026 18:03:50 -0500 Subject: [PATCH 2/2] Retire tsc now that vp check type checks --- AGENTS.md | 5 +---- REVIEW.md | 4 ++-- docs/agents/testing.md | 2 +- package.json | 4 +--- packages/graph-explorer-proxy-server/package.json | 1 - packages/graph-explorer/package.json | 1 - packages/shared/package.json | 3 --- 7 files changed, 5 insertions(+), 15 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 726f585b5..0353f447c 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -75,9 +75,8 @@ Pass `REVIEW.md` by path to every reviewer sub-agent, not only the standards one ## Commands -Run from project root. Prefer these `pnpm` scripts. `vp` subcommands (`vp test`, `vp lint`, `vp fmt`, `vp check`) are fine to run directly, since `vp` resolves the project's pinned Vite+ and its config; use `pnpm exec vp` if the global CLI isn't installed. `vp check` runs format, lint, and type checks. Never invoke `tsc`, `vitest`, `oxlint`, or `oxfmt` directly or via `pnpx`, because bare tools use the wrong version and miss project context. +Run from project root. Prefer these `pnpm` scripts. `vp` subcommands (`vp test`, `vp lint`, `vp fmt`, `vp check`) are fine to run directly, since `vp` resolves the project's pinned Vite+ and its config; use `pnpm exec vp` if the global CLI isn't installed. `vp check` runs format, lint, and type checks. Never invoke `tsc`, `tsgo`, `vitest`, `oxlint`, or `oxfmt` directly or via `pnpx`, because bare tools use the wrong version and miss project context. -- `pnpm check:types` — typecheck all packages (no per-file/per-package option; this is the granularity) - `pnpm checks` — all static checks (types + lint + format); default validation for small changes - `pnpm check:lint` / `pnpm lint` — lint / lint and fix - `pnpm check:format` / `pnpm format` — check / fix formatting @@ -85,8 +84,6 @@ Run from project root. Prefer these `pnpm` scripts. `vp` subcommands (`vp test`, - `pnpm test ` — test files matching a path substring (file, dir, or partial) - `pnpm test -t "suite > name"` — test by name, with segments joined by a spaced `>` exactly as the reporter prints them; a single segment such as `-t "renders empty state"` also works (no `--` separator; pnpm forwards args verbatim, unlike npm) -`pnpm check:types` runs in parallel across packages in under a minute; just run it. - ## Agent skills ### Issue tracker diff --git a/REVIEW.md b/REVIEW.md index 620c8b536..165a556e0 100644 --- a/REVIEW.md +++ b/REVIEW.md @@ -12,7 +12,7 @@ Reserve Important for the cases below. Everything else is Nit at most. That incl - Values that reach a query outside a Query Fragment, or any other break of the rules in `docs/agents/connectors.md`. - A limit that doesn't bound the scan. - A change to one connector, or to shared query code, that changes behavior in the other query languages without saying why. -- **Runtime APIs newer than the browser floor in `packages/graph-explorer/.browserslistrc`,** such as `Error.isError` or Iterator helpers. `tsc` and oxlint catch most of these. Check by hand for what they miss, listed in `docs/agents/browser-floor.md`: DOM instance members, and built-in features that `lib` allows but the floor doesn't. +- **Runtime APIs newer than the browser floor in `packages/graph-explorer/.browserslistrc`,** such as `Error.isError` or Iterator helpers. The type check and oxlint catch most of these. Check by hand for what they miss, listed in `docs/agents/browser-floor.md`: DOM instance members, and built-in features that `lib` allows but the floor doesn't. - **Production code that imports a `devDependency`.** - **Work that grows with graph or schema size:** one query or subscription per item, quadratic loops, or copying passes over large collections. - **Tests that can't fail.** Name a plausible regression of the behavior the test's name promises, and prove the verdict with a mutation: inject that regression and run the test. A test that stays green is the finding, and the fix is to tighten it until the mutation turns it red. Recommend deletion only when no regression of that behavior could reach the test. Flag these: @@ -32,7 +32,7 @@ Reserve Important for the cases below. Everything else is Nit at most. That incl ## Do not report -- Anything CI enforces: `vp lint` (oxlint, including the React Compiler rules), `vp fmt` (oxfmt), `tsc`, lockfile checks, Trivy and dependency advisories. +- Anything CI enforces: `vp lint` (oxlint, including the React Compiler rules), `vp fmt` (oxfmt), the type check in `vp check`, lockfile checks, Trivy and dependency advisories. - Pre-existing issues, unless this PR makes them newly reachable. - The Git rules in `AGENTS.md`. PRs are squash merged, so commit messages, branch names, and how the commits are split never reach `main`. - `Changelog.md`. It's updated at release time. diff --git a/docs/agents/testing.md b/docs/agents/testing.md index 00fc62f5e..a55b1fd5e 100644 --- a/docs/agents/testing.md +++ b/docs/agents/testing.md @@ -76,7 +76,7 @@ Commands are in AGENTS.md. - Small change: `pnpm checks`, plus `pnpm test ` for the area you touched - Before a PR, or after touching shared utilities, core providers, or type definitions: `pnpm test` -- No associated tests (config or type-only change): `pnpm check:types` +- No associated tests (config or type-only change): `pnpm checks` ## Special cases diff --git a/package.json b/package.json index 10ed5300f..2fcb5a260 100644 --- a/package.json +++ b/package.json @@ -14,9 +14,7 @@ "check:format": "vp fmt --check", "test": "vp test run", "test:watch": "vp test watch", - "check:types": "pnpm -r --include-workspace-root --parallel --stream run typecheck", - "typecheck": "tsc --noEmit", - "checks": "vp check && pnpm run check:types", + "checks": "vp check", "start": "pnpm --filter \"graph-explorer-proxy-server\" run start", "clean": "pnpm --stream -r run clean", "clean:dep": "rm -rf node_modules && pnpm -r exec rm -rf node_modules", diff --git a/packages/graph-explorer-proxy-server/package.json b/packages/graph-explorer-proxy-server/package.json index 54156a37e..de8e10a1c 100644 --- a/packages/graph-explorer-proxy-server/package.json +++ b/packages/graph-explorer-proxy-server/package.json @@ -6,7 +6,6 @@ "author": "amazon", "type": "module", "scripts": { - "typecheck": "tsc --noEmit", "dev": "node src/node-server.ts", "start": "NODE_ENV=production node src/node-server.ts" }, diff --git a/packages/graph-explorer/package.json b/packages/graph-explorer/package.json index 6208c3c7a..912c22e7a 100644 --- a/packages/graph-explorer/package.json +++ b/packages/graph-explorer/package.json @@ -6,7 +6,6 @@ "author": "amazon", "type": "module", "scripts": { - "typecheck": "tsc --noEmit", "dev": "vp dev --force", "clean": "rimraf dist", "vite-build": "NODE_OPTIONS=--max_old_space_size=6144 vp build", diff --git a/packages/shared/package.json b/packages/shared/package.json index 9801f388d..c8595e77a 100644 --- a/packages/shared/package.json +++ b/packages/shared/package.json @@ -12,9 +12,6 @@ "./types": "./src/types/index.ts", "./utils/testing": "./src/utils/testing/index.ts" }, - "scripts": { - "typecheck": "tsc --noEmit" - }, "devDependencies": { "vite-plus": "catalog:" }