From c0501151b51382536d61d5108ec31ebfae6488da Mon Sep 17 00:00:00 2001 From: arandomogg Date: Thu, 20 Aug 2026 02:27:00 +0100 Subject: [PATCH] Fail production builds on TypeScript errors next.config.mjs set typescript.ignoreBuildErrors, so `next build` emitted deployment artifacts even when the project did not type-check. On a codebase spanning payments, KYC, contract, and admin boundaries, that turned type drift into runtime failures against real records instead of a failed build. Removing the flag surfaced eleven pre-existing errors, all from one source: the handler `defineRoute` returns declared its Next.js context argument as optional and loosely typed, which does not satisfy the RouteContext contract Next generates under .next/types. The argument is now the exported NextRouteContext type, declared exactly as the framework passes it. Callers that construct a handler directly pass an explicit empty-params context. Gate changes: - `npm run typecheck` runs with --incremental false so a stale .tsbuildinfo cannot let a check pass by reusing an earlier result. - `npm run typecheck:gate` (scripts/check-typecheck-gate.ts) fails if the suppression flags return or a file-level nocheck directive appears, and proves the gate still bites by introducing a deliberate type error and requiring tsc to reject it. - CI runs the gate on pull requests and on main, before the build step. Compiler strictness is unchanged: no blanket ignore, no file-level nocheck, and no relaxed tsconfig option replaces the removed flag. docs/type-safety.md records the gate, its stages, and the one standing exception (skipLibCheck, which covers only third-party declaration files in node_modules). --- .github/workflows/ci.yml | 6 + .gitignore | 2 + CONTRIBUTING.md | 7 +- __tests__/lib/api/route-handler.test.ts | 59 +++++---- __tests__/lib/api/route-serialization.test.ts | 28 ++-- docs/type-safety.md | 70 ++++++++++ lib/api/route-handler.ts | 21 ++- next.config.mjs | 6 +- package.json | 3 +- scripts/check-typecheck-gate.ts | 121 ++++++++++++++++++ 10 files changed, 280 insertions(+), 43 deletions(-) create mode 100644 docs/type-safety.md create mode 100644 scripts/check-typecheck-gate.ts diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index c6f3a47d..8cd8ddbf 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -52,6 +52,9 @@ jobs: - name: TypeScript check run: npm run typecheck + - name: TypeScript build gate + run: npm run typecheck:gate + - name: API contract tests run: npm run test:contracts @@ -104,6 +107,9 @@ jobs: - name: TypeScript check run: npm run typecheck + - name: TypeScript build gate + run: npm run typecheck:gate + - name: API contract tests run: npm run test:contracts diff --git a/.gitignore b/.gitignore index f38bb77f..41828526 100644 --- a/.gitignore +++ b/.gitignore @@ -27,6 +27,8 @@ yarn-error.log* # typescript *.tsbuildinfo next-env.d.ts +# scratch file written by scripts/check-typecheck-gate.ts +lib/__typecheck-gate-probe__.ts # rust /target diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 71d89641..24b3c371 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -57,10 +57,14 @@ Run: ```bash npm run lint -npx tsc --noEmit +npm run typecheck npm run build ``` +`next build` fails on any TypeScript error, and CI runs the same checks. Do not +disable them with `typescript.ignoreBuildErrors` or a file-level `@ts-nocheck` — +see [docs/type-safety.md](docs/type-safety.md). + ## PR scope Keep pull requests small and clear. In your PR description, state whether your change affects: @@ -148,6 +152,7 @@ Before opening a PR run: ```bash npm run lint +npm run typecheck npm run build ``` diff --git a/__tests__/lib/api/route-handler.test.ts b/__tests__/lib/api/route-handler.test.ts index 6205089a..4630b3d0 100644 --- a/__tests__/lib/api/route-handler.test.ts +++ b/__tests__/lib/api/route-handler.test.ts @@ -2,6 +2,8 @@ import { beforeEach, describe, expect, it, vi } from "vitest" import { z } from "zod" +import type { NextRouteContext } from "@/lib/api/route-handler" + const getAuthenticatedUser = vi.fn() const withSessionRefresh = vi.fn(async (response: unknown, _user: unknown) => response) const logAuthorizationDenial = vi.fn(async (_input: unknown) => undefined) @@ -19,6 +21,12 @@ const { defineRoute } = await import("@/lib/api/route-handler") const { ApiError } = await import("@/lib/api/errors") const { DEPRECATED_API_VERSIONS } = await import("@/lib/api/versioning") +/** + * Next.js always passes a route context; routes without dynamic segments + * simply receive empty params. Tests mirror that call shape. + */ +const noParams: NextRouteContext = { params: Promise.resolve({}) } + const INVESTOR_ID = "665f1a2b3c4d5e6f70819203" const OTHER_ID = "665f1a2b3c4d5e6f70819999" @@ -57,7 +65,7 @@ describe("defineRoute — request validation", () => { }) it("rejects malformed JSON with a stable code rather than a parser message", async () => { - const response = await route(jsonRequest("{ not json")) + const response = await route(jsonRequest("{ not json"), noParams) const payload = await response.json() expect(response.status).toBe(400) @@ -68,7 +76,7 @@ describe("defineRoute — request validation", () => { }) it("returns field-level errors for an invalid body", async () => { - const response = await route(jsonRequest({ amount: -5 })) + const response = await route(jsonRequest({ amount: -5 }), noParams) const payload = await response.json() expect(response.status).toBe(400) @@ -79,7 +87,7 @@ describe("defineRoute — request validation", () => { }) it("reports unknown body fields instead of silently dropping them", async () => { - const response = await route(jsonRequest({ amount: 5, exchangeRate: 1500 })) + const response = await route(jsonRequest({ amount: 5, exchangeRate: 1500 }), noParams) const payload = await response.json() expect(response.status).toBe(400) @@ -89,6 +97,7 @@ describe("defineRoute — request validation", () => { it("rejects a non-JSON content type", async () => { const response = await route( jsonRequest("amount=5", { headers: { "content-type": "application/x-www-form-urlencoded" } }), + noParams, ) expect(response.status).toBe(415) @@ -105,7 +114,7 @@ describe("defineRoute — request validation", () => { handler: async () => ({ success: true as const, value: "ok" }), }) - const response = await queryRoute(new Request("https://chainmove.test/api/thing?page=abc")) + const response = await queryRoute(new Request("https://chainmove.test/api/thing?page=abc"), noParams) const payload = await response.json() expect(response.status).toBe(400) @@ -129,7 +138,7 @@ describe("defineRoute — authentication and authorization", () => { it("returns 401 when unauthenticated", async () => { authenticateAs(null) - const response = await ownedRoute(new Request("https://chainmove.test/api/thing")) + const response = await ownedRoute(new Request("https://chainmove.test/api/thing"), noParams) expect(response.status).toBe(401) expect((await response.json()).code).toBe("UNAUTHENTICATED") @@ -147,7 +156,7 @@ describe("defineRoute — authentication and authorization", () => { handler: async () => ({ success: true as const, value: "ok" }), }) - const response = await adminRoute(new Request("https://chainmove.test/api/thing")) + const response = await adminRoute(new Request("https://chainmove.test/api/thing"), noParams) expect(response.status).toBe(403) expect((await response.json()).code).toBe("FORBIDDEN") @@ -164,7 +173,7 @@ describe("defineRoute — authentication and authorization", () => { handler: async () => ({ success: true as const, value: "ok" }), }) - const response = await foreignRoute(new Request("https://chainmove.test/api/thing")) + const response = await foreignRoute(new Request("https://chainmove.test/api/thing"), noParams) const payload = await response.json() expect(response.status).toBe(404) @@ -186,7 +195,7 @@ describe("defineRoute — authentication and authorization", () => { handler, }) - const response = await missingRoute(new Request("https://chainmove.test/api/thing")) + const response = await missingRoute(new Request("https://chainmove.test/api/thing"), noParams) expect(response.status).toBe(404) expect(handler).not.toHaveBeenCalled() @@ -259,7 +268,7 @@ describe("defineRoute — error mapping", () => { it("never leaks an unexpected error message", async () => { const route = routeThrowing(new Error("connect ECONNREFUSED mongodb://user:pa55w0rd@10.0.0.5:27017")) - const response = await route(new Request("https://chainmove.test/api/thing")) + const response = await route(new Request("https://chainmove.test/api/thing"), noParams) const payload = await response.json() expect(response.status).toBe(500) @@ -273,7 +282,7 @@ describe("defineRoute — error mapping", () => { const error = new Error("boom") error.stack = "Error: boom\n at /srv/chainmove/lib/services/investments.service.ts:142:9" - const response = await routeThrowing(error)(new Request("https://chainmove.test/api/thing")) + const response = await routeThrowing(error)(new Request("https://chainmove.test/api/thing"), noParams) const body = JSON.stringify(await response.json()) expect(body).not.toContain("at /srv") @@ -286,7 +295,7 @@ describe("defineRoute — error mapping", () => { Object.assign(new Error("Transaction aborted"), { errorLabels: ["TransientTransactionError"] }), ) - const response = await route(new Request("https://chainmove.test/api/thing")) + const response = await route(new Request("https://chainmove.test/api/thing"), noParams) expect(response.status).toBe(503) expect((await response.json()).code).toBe("TRANSIENT_CONFLICT") @@ -297,7 +306,7 @@ describe("defineRoute — error mapping", () => { Object.assign(new Error("E11000 duplicate key error collection: users index: email_1"), { code: 11000 }), ) - const response = await route(new Request("https://chainmove.test/api/thing")) + const response = await route(new Request("https://chainmove.test/api/thing"), noParams) const payload = await response.json() expect(response.status).toBe(409) @@ -311,7 +320,7 @@ describe("defineRoute — error mapping", () => { }), ) - const response = await route(new Request("https://chainmove.test/api/thing")) + const response = await route(new Request("https://chainmove.test/api/thing"), noParams) const payload = await response.json() expect(response.status).toBe(502) @@ -322,7 +331,7 @@ describe("defineRoute — error mapping", () => { it("attaches a correlation id to every error", async () => { const route = routeThrowing(new Error("boom")) - const response = await route(new Request("https://chainmove.test/api/thing")) + const response = await route(new Request("https://chainmove.test/api/thing"), noParams) const payload = await response.json() expect(payload.correlationId).toMatch(/^[0-9a-f-]{36}$/) @@ -334,6 +343,7 @@ describe("defineRoute — error mapping", () => { const response = await route( new Request("https://chainmove.test/api/thing", { headers: { "x-correlation-id": "edge-abc-123" } }), + noParams, ) expect((await response.json()).correlationId).toBe("edge-abc-123") @@ -356,7 +366,7 @@ describe("defineRoute — response serialization", () => { }) as never, }) - const response = await route(new Request("https://chainmove.test/api/thing")) + const response = await route(new Request("https://chainmove.test/api/thing"), noParams) const payload = await response.json() expect(payload).toEqual({ success: true, value: "ok" }) @@ -372,7 +382,7 @@ describe("defineRoute — response serialization", () => { handler: async () => ({ success: true as const }) as never, }) - const response = await route(new Request("https://chainmove.test/api/thing")) + const response = await route(new Request("https://chainmove.test/api/thing"), noParams) const payload = await response.json() expect(response.status).toBe(500) @@ -393,7 +403,7 @@ describe("defineRoute — response serialization", () => { }), }) - const response = await route(new Request("https://chainmove.test/api/thing")) + const response = await route(new Request("https://chainmove.test/api/thing"), noParams) expect(response.status).toBe(500) expect((await response.json()).code).toBe("INTERNAL_ERROR") @@ -411,7 +421,7 @@ describe("defineRoute — response serialization", () => { handler: async () => ({ success: true as const, value: "ok" }), }) - const response = await route(jsonRequest({})) + const response = await route(jsonRequest({}), noParams) expect(response.status).toBe(201) expect(withSessionRefresh).toHaveBeenCalledTimes(1) @@ -429,7 +439,7 @@ describe("defineRoute — versioning and deprecation", () => { }) it("reports the serving version on success", async () => { - const response = await route(new Request("https://chainmove.test/api/thing")) + const response = await route(new Request("https://chainmove.test/api/thing"), noParams) expect(response.status).toBe(200) expect(response.headers.get("X-API-Version")).toBe("2026-01-01") @@ -438,6 +448,7 @@ describe("defineRoute — versioning and deprecation", () => { it("honours a supported pinned version", async () => { const response = await route( new Request("https://chainmove.test/api/thing", { headers: { "X-API-Version": "2026-01-01" } }), + noParams, ) expect(response.status).toBe(200) @@ -446,6 +457,7 @@ describe("defineRoute — versioning and deprecation", () => { it("rejects an unsupported version with a stable code", async () => { const response = await route( new Request("https://chainmove.test/api/thing", { headers: { "X-API-Version": "1999-01-01" } }), + noParams, ) const payload = await response.json() @@ -470,7 +482,7 @@ describe("defineRoute — versioning and deprecation", () => { handler: async () => ({ success: true as const, value: "ok" }), }) - const response = await deprecated(new Request("https://chainmove.test/api/thing")) + const response = await deprecated(new Request("https://chainmove.test/api/thing"), noParams) expect(response.headers.get("Deprecation")).toContain("2026") expect(response.headers.get("Sunset")).toContain("2026") @@ -491,6 +503,7 @@ describe("defineRoute — versioning and deprecation", () => { try { const response = await route( new Request("https://chainmove.test/api/thing", { headers: { "X-API-Version": "2026-01-01" } }), + noParams, ) // A deprecated version still works until its sunset date. @@ -526,7 +539,7 @@ describe("defineRoute — versioning and deprecation", () => { }) try { - const response = await deprecatedEndpoint(new Request("https://chainmove.test/api/thing")) + const response = await deprecatedEndpoint(new Request("https://chainmove.test/api/thing"), noParams) // The endpoint notice is the more specific signal for this caller. expect(response.headers.get("Link")).toContain("docs/endpoint") @@ -537,7 +550,7 @@ describe("defineRoute — versioning and deprecation", () => { }) it("sends no deprecation headers on a current version", async () => { - const response = await route(new Request("https://chainmove.test/api/thing")) + const response = await route(new Request("https://chainmove.test/api/thing"), noParams) expect(response.headers.get("Deprecation")).toBeNull() expect(response.headers.get("Sunset")).toBeNull() @@ -554,7 +567,7 @@ describe("defineRoute — versioning and deprecation", () => { handler: async () => ({ success: true as const, value: "ok" }), }) - const response = await paginated(new Request("https://chainmove.test/api/thing?limit=10")) + const response = await paginated(new Request("https://chainmove.test/api/thing?limit=10"), noParams) expect(response.headers.get("Warning")).toContain("'limit' query parameter is deprecated") }) diff --git a/__tests__/lib/api/route-serialization.test.ts b/__tests__/lib/api/route-serialization.test.ts index e7e4868b..9ebd9f0b 100644 --- a/__tests__/lib/api/route-serialization.test.ts +++ b/__tests__/lib/api/route-serialization.test.ts @@ -1,6 +1,8 @@ // @vitest-environment node import { beforeEach, describe, expect, it, vi } from "vitest" +import type { NextRouteContext } from "@/lib/api/route-handler" + /** * End-to-end serialization tests for the converted routes. * @@ -64,6 +66,12 @@ function chainable(result: unknown) { return chain } +/** + * Next.js always passes a route context; routes without dynamic segments + * simply receive empty params. Tests mirror that call shape. + */ +const noParams: NextRouteContext = { params: Promise.resolve({}) } + const INVESTOR_ID = "665f1a2b3c4d5e6f70819203" function authenticateAs(role: string, overrides: Record = {}) { @@ -104,7 +112,7 @@ describe("GET /api/wallet/summary", () => { ) const { GET } = await import("@/app/api/wallet/summary/route") - const response = await GET(new Request("https://chainmove.test/api/wallet/summary")) + const response = await GET(new Request("https://chainmove.test/api/wallet/summary"), noParams) const body = await response.json() expect(response.status).toBe(200) @@ -135,7 +143,7 @@ describe("GET /api/wallet/summary", () => { ) const { GET } = await import("@/app/api/wallet/summary/route") - const body = await (await GET(new Request("https://chainmove.test/api/wallet/summary"))).json() + const body = await (await GET(new Request("https://chainmove.test/api/wallet/summary"), noParams)).json() const serialized = JSON.stringify(body) expect(serialized).not.toContain("10.0.0.5") @@ -164,7 +172,7 @@ describe("GET /api/investments", () => { ) const { GET } = await import("@/app/api/investments/route") - const response = await GET(new Request("https://chainmove.test/api/investments")) + const response = await GET(new Request("https://chainmove.test/api/investments"), noParams) const body = await response.json() expect(response.status).toBe(200) @@ -205,7 +213,7 @@ describe("GET /api/pools", () => { ]) const { GET } = await import("@/app/api/pools/route") - const response = await GET(new Request("https://chainmove.test/api/pools")) + const response = await GET(new Request("https://chainmove.test/api/pools"), noParams) const body = await response.json() expect(response.status).toBe(200) @@ -242,7 +250,7 @@ describe("GET /api/pools", () => { ]) const { GET } = await import("@/app/api/pools/route") - const body = await (await GET(new Request("https://chainmove.test/api/pools"))).json() + const body = await (await GET(new Request("https://chainmove.test/api/pools"), noParams)).json() // The page renders `userInvested?.amountMajor || 0`, so absent is safe. expect(body.pools[0].userInvested).toBeUndefined() @@ -279,7 +287,7 @@ describe("GET /api/transactions/ledger", () => { it("serializes entries, pagination, and summary the ledger table reads", async () => { const { GET } = await import("@/app/api/transactions/ledger/route") - const response = await GET(new Request("https://chainmove.test/api/transactions/ledger?page=1&pageSize=20")) + const response = await GET(new Request("https://chainmove.test/api/transactions/ledger?page=1&pageSize=20"), noParams) const body = await response.json() expect(response.status).toBe(200) @@ -296,7 +304,7 @@ describe("GET /api/transactions/ledger", () => { it("drops raw provider metadata", async () => { const { GET } = await import("@/app/api/transactions/ledger/route") - const body = await (await GET(new Request("https://chainmove.test/api/transactions/ledger"))).json() + const body = await (await GET(new Request("https://chainmove.test/api/transactions/ledger"), noParams)).json() expect(body.transactions[0].metadata).toBeUndefined() expect(JSON.stringify(body)).not.toContain("10.0.0.5") @@ -307,7 +315,7 @@ describe("GET /api/transactions/ledger", () => { authenticateAs("admin") const { GET } = await import("@/app/api/transactions/ledger/route") - const body = await (await GET(new Request("https://chainmove.test/api/transactions/ledger"))).json() + const body = await (await GET(new Request("https://chainmove.test/api/transactions/ledger"), noParams)).json() expect(body.scope).toBe("global") }) @@ -338,7 +346,7 @@ describe("GET /api/admin/kyc-requests", () => { userCount.mockResolvedValue(1) const { GET } = await import("@/app/api/admin/kyc-requests/route") - const response = await GET(new Request("https://chainmove.test/api/admin/kyc-requests")) + const response = await GET(new Request("https://chainmove.test/api/admin/kyc-requests"), noParams) const body = await response.json() expect(response.status).toBe(200) @@ -360,7 +368,7 @@ describe("GET /api/admin/kyc-requests", () => { authenticateAs("investor") const { GET } = await import("@/app/api/admin/kyc-requests/route") - const response = await GET(new Request("https://chainmove.test/api/admin/kyc-requests")) + const response = await GET(new Request("https://chainmove.test/api/admin/kyc-requests"), noParams) expect(response.status).toBe(403) }) diff --git a/docs/type-safety.md b/docs/type-safety.md new file mode 100644 index 00000000..11053175 --- /dev/null +++ b/docs/type-safety.md @@ -0,0 +1,70 @@ +# TypeScript build gate + +Production builds fail on TypeScript errors. `next.config.mjs` no longer sets +`typescript.ignoreBuildErrors`, so `next build` type-checks the whole project — +including the route contracts Next.js generates under `.next/types` — before it +emits any deployment artifact. + +## Why the flag mattered + +The codebase spans money movement, KYC, on-chain contract calls, and admin +tooling. With the flag on, a handler whose signature drifted from the framework +contract, or a service returning a shape its caller no longer expects, still +produced a green build and shipped. The type error surfaced as a runtime failure +against real user records instead of as a failed build. + +Removing the flag immediately surfaced eleven such errors: every route built +with `defineRoute` declared its Next.js context argument as optional and loosely +typed, which does not satisfy the generated `RouteContext` contract. That is now +`NextRouteContext` in `lib/api/route-handler.ts`, declared exactly as the +framework passes it. + +## The gate + +| Stage | Command | Covers | +| --- | --- | --- | +| Local, pre-PR | `npm run typecheck` | All `.ts`/`.tsx` sources, including tests | +| CI | `npm run typecheck` | Same, deterministically (incremental cache disabled) | +| CI | `npm run typecheck:gate` | Proves the gate still rejects a type error | +| CI | `npm run build` | Sources plus generated route and page types | + +`npm run typecheck` passes `--incremental false`. A stale `.tsbuildinfo` can +otherwise let a check pass by reusing an earlier result, which makes the gate +non-deterministic across machines and CI caches. + +`npm run typecheck` runs before `.next/types` exists, so it cannot see the +generated route contracts. `npm run build` is the step that checks those, and it +runs on every pull request; neither step substitutes for the other. + +## Verifying the gate + +`npm run typecheck:gate` (`scripts/check-typecheck-gate.ts`) asserts two things: + +1. `next.config.mjs` does not enable `typescript.ignoreBuildErrors` or + `eslint.ignoreDuringBuilds`. +2. No tracked `.ts`/`.tsx` file carries a file-level `@ts-nocheck`, which would + exempt a whole file from the gate. +3. Introducing a deliberate type error makes the typecheck fail. The script + writes a probe file under `lib/`, runs `tsc`, requires a non-zero exit that + names the probe, and deletes the probe again. + +The third check is what keeps the gate honest: it fails if compiler strictness +or the `tsconfig.json` include list is weakened to the point where the type +error would no longer be seen. + +## Suppressions + +Strictness stays as configured in `tsconfig.json` (`strict: true`). Do not: + +- reintroduce `typescript.ignoreBuildErrors` or `eslint.ignoreDuringBuilds`, +- add `@ts-nocheck` to a source file, +- relax a `tsconfig.json` compiler option to clear an error. + +`skipLibCheck: true` remains the one standing exception. It applies only to +declaration files inside `node_modules` — third-party generated typings the +repository cannot fix — and does not weaken checking of any first-party code. + +If a type is genuinely unrepresentable, narrow the suppression to the single +line with `@ts-expect-error` and a comment explaining why. `@ts-expect-error` +itself errors once the underlying problem is fixed, so the suppression cannot +outlive its reason. diff --git a/lib/api/route-handler.ts b/lib/api/route-handler.ts index fb6a3a02..077ab3c6 100644 --- a/lib/api/route-handler.ts +++ b/lib/api/route-handler.ts @@ -35,6 +35,18 @@ type AuthMode = "public" | "authenticated" | "webhook" type AuthenticatedUser = { _id: unknown; role?: unknown; [key: string]: unknown } +/** + * Second argument Next.js passes to an App Router route handler. + * + * This mirrors the `RouteContext` shape in the generated `.next/types` route + * contracts — required, with `params` always a promise — so `next build` + * accepts every handler `defineRoute` produces. Widening it (for example back + * to an optional argument) makes the generated contract check fail. + */ +export type NextRouteContext = { + params: Promise> +} + export interface RouteContext { request: Request params: TParams @@ -106,10 +118,7 @@ export function defineRoute< TResponseSchema extends z.ZodTypeAny = z.ZodTypeAny, TAuth extends AuthMode = "authenticated", >(definition: RouteDefinition) { - return async function routeHandler( - request: Request, - nextContext?: { params?: Promise> | Record }, - ): Promise { + return async function routeHandler(request: Request, nextContext: NextRouteContext): Promise { const correlationId = resolveCorrelationId(request) const extraHeaders: Record = {} let successStatus = definition.successStatus ?? (definition.method === "POST" ? 201 : 200) @@ -181,7 +190,9 @@ export class NoContent {} async function parseParams( definition: { params?: z.ZodTypeAny }, - nextContext?: { params?: Promise> | Record }, + // Optional here, not in the exported handler signature: direct callers such + // as tests may omit the context entirely, while Next.js always supplies it. + nextContext: NextRouteContext | undefined, ) { if (!definition.params) return undefined diff --git a/next.config.mjs b/next.config.mjs index 38746186..f8b70fe0 100644 --- a/next.config.mjs +++ b/next.config.mjs @@ -2,9 +2,9 @@ import path from "path" /** @type {import('next').NextConfig} */ const nextConfig = { - typescript: { - ignoreBuildErrors: true, - }, + // Type errors must fail `next build`. Do not reintroduce + // `typescript.ignoreBuildErrors` or `eslint.ignoreDuringBuilds` — + // see docs/type-safety.md. images: { unoptimized: true, }, diff --git a/package.json b/package.json index 0aa555c7..a241e7b0 100644 --- a/package.json +++ b/package.json @@ -9,7 +9,8 @@ "lint": "eslint .", "test": "vitest run", "test:contracts": "vitest run __tests__/lib/api", - "typecheck": "tsc --noEmit", + "typecheck": "tsc --noEmit --incremental false", + "typecheck:gate": "tsx scripts/check-typecheck-gate.ts", "openapi:generate": "tsx scripts/generate-openapi.ts", "openapi:check": "tsx scripts/check-openapi-drift.ts && tsx scripts/check-openapi-compat.ts", "start": "next start", diff --git a/scripts/check-typecheck-gate.ts b/scripts/check-typecheck-gate.ts new file mode 100644 index 00000000..2646d5f6 --- /dev/null +++ b/scripts/check-typecheck-gate.ts @@ -0,0 +1,121 @@ +/** + * Proves the TypeScript build gate is real. + * + * Type errors used to be waved through by `typescript.ignoreBuildErrors` in + * next.config.mjs, so a broken API contract could ship. This script is the + * regression test for that: it fails CI if the escape hatch comes back, and it + * proves the gate still bites by deliberately introducing a type error and + * asserting that `tsc` rejects it. + */ +import { execFileSync } from "child_process" +import { existsSync, readFileSync, rmSync, writeFileSync } from "fs" +import { createRequire } from "module" +import { dirname, resolve } from "path" + +const require = createRequire(import.meta.url) +const typescriptCli = resolve(dirname(require.resolve("typescript/package.json")), "bin/tsc") + +/** Config keys that would silently disable a build gate if reintroduced. */ +const FORBIDDEN_CONFIG = [ + { pattern: /ignoreBuildErrors\s*:\s*true/, name: "typescript.ignoreBuildErrors" }, + { pattern: /ignoreDuringBuilds\s*:\s*true/, name: "eslint.ignoreDuringBuilds" }, +] + +/** + * Written, compiled, and deleted by this script. It lives under `lib/` so it is + * covered by the project's TypeScript include globs — a probe outside the + * compiler's view would pass no matter how broken the gate is. + */ +const PROBE_PATH = "lib/__typecheck-gate-probe__.ts" +const PROBE_SOURCE = `// Generated by scripts/check-typecheck-gate.ts. Deleted again before the script exits. +export const deliberateTypeError: string = 42 +` + +function checkNextConfig(): void { + const config = readFileSync("next.config.mjs", "utf8") + + for (const { pattern, name } of FORBIDDEN_CONFIG) { + if (pattern.test(config)) { + throw new Error( + `next.config.mjs enables ${name}. Production builds must fail on type errors; ` + + "fix the offending types instead of suppressing the gate.", + ) + } + } + + console.log("next.config.mjs declares no build-error suppression.") +} + +/** + * A file-level nocheck directive exempts a whole file from the gate — the same + * failure mode as `ignoreBuildErrors`, with a narrower blast radius. Only a + * line-scoped `@ts-expect-error`, which itself errors once the underlying + * problem is fixed, is an acceptable suppression. + * + * The directive is assembled rather than written out so this script does not + * match itself and does not need to be exempted from its own check. + */ +const NOCHECK_DIRECTIVE = ["@ts", "nocheck"].join("-") + +function checkNoFileLevelSuppressions(): void { + const tracked = execFileSync("git", ["ls-files", "*.ts", "*.tsx"], { encoding: "utf8" }) + .split(/\r?\n/) + .map((file) => file.trim()) + .filter(Boolean) + + const suppressed = tracked.filter((file) => readFileSync(file, "utf8").includes(NOCHECK_DIRECTIVE)) + + if (suppressed.length > 0) { + throw new Error( + `${NOCHECK_DIRECTIVE} exempts whole files from the gate. Remove it from: ${suppressed.join(", ")}. ` + + "Use a line-scoped @ts-expect-error with a comment if a type is genuinely unrepresentable.", + ) + } + + console.log(`No ${NOCHECK_DIRECTIVE} directive in ${tracked.length} tracked TypeScript files.`) +} + +function runTypecheck(): { ok: boolean; output: string } { + try { + const output = execFileSync(process.execPath, [typescriptCli, "--noEmit", "--incremental", "false"], { + encoding: "utf8", + stdio: ["ignore", "pipe", "pipe"], + }) + return { ok: true, output } + } catch (error) { + const failure = error as { stdout?: string; stderr?: string } + return { ok: false, output: `${failure.stdout ?? ""}${failure.stderr ?? ""}` } + } +} + +function checkGateRejectsTypeErrors(): void { + if (existsSync(PROBE_PATH)) { + throw new Error(`${PROBE_PATH} already exists. Remove it — it is a scratch file, never a committed one.`) + } + + writeFileSync(PROBE_PATH, PROBE_SOURCE) + + try { + const { ok, output } = runTypecheck() + + if (ok) { + throw new Error( + "The typecheck gate accepted a deliberate type error. Compiler strictness or the " + + "tsconfig include list has been weakened.", + ) + } + + if (!output.includes("__typecheck-gate-probe__")) { + throw new Error(`The typecheck failed, but not on the probe file. Output:\n${output}`) + } + + console.log("Typecheck rejects an introduced type error, as expected.") + } finally { + rmSync(PROBE_PATH, { force: true }) + } +} + +checkNextConfig() +checkNoFileLevelSuppressions() +checkGateRejectsTypeErrors() +console.log("TypeScript build gate verified.")