From 93ac78a5ed77e99f0478ebfdb030c9ba4df00054 Mon Sep 17 00:00:00 2001 From: Jack Zhuang <277994282+os-zhuang@users.noreply.github.com> Date: Tue, 28 Jul 2026 20:10:51 +0800 Subject: [PATCH] =?UTF-8?q?chore(type-check):=20site=20is=20not=20debt=20?= =?UTF-8?q?=E2=80=94=20its=20own=20`next=20build`=20type-checks=20it?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The #2911 sweep recorded `@object-ui/site` as 7 errors of debt. Measured: it is zero. The 7 TS2304 on `LayoutProps`/`PageProps`/`RouteContext` were entirely an artifact of never having built it — `apps/site/tsconfig.json` includes `.next/types/**/*.ts`, and those types only exist after `next build`. Run the build first and `tsc --noEmit` is exit 0. So site was never broken; it was miscategorised, and its phantom 7 inflated the outstanding-error count by 28% (32 -> 25). It also does not need a `type-check` script: `next build` runs a full type-check unless `typescript.ignoreBuildErrors` is set, and the `docs` CI job runs that build. Adding a script would just run the compiler twice — and would pull a Next build into the Type Check job. New `CHECKED_BY_OWN_BUILD` category records that, and — because `ignoreBuildErrors` is precisely how such an exemption would silently rot into the #2911 hole — verifies it rather than trusting it. Three failure modes, all proven red before being trusted: - `ignoreBuildErrors: true` added to next.config.mjs -> exit 1 - site gains a `type-check` script (stale entry) -> exit 1 - build script changes out from under the exemption -> exit 1 Recorded caveat, since it is a real cost/coverage call rather than an oversight: the `docs` job runs the site build only when `apps/site/` or `content/` changed (plus every push to main), so a PR touching only a package in `transpilePackages` does not re-check the site until it lands. No changeset: scripts-only, no published package changes. Refs #2911, #2915, #2919 Co-Authored-By: Claude --- scripts/check-type-check-coverage.mjs | 69 +++++++++++++++++++++++++-- 1 file changed, 66 insertions(+), 3 deletions(-) diff --git a/scripts/check-type-check-coverage.mjs b/scripts/check-type-check-coverage.mjs index 142c5db232..f59e25cf15 100644 --- a/scripts/check-type-check-coverage.mjs +++ b/scripts/check-type-check-coverage.mjs @@ -30,7 +30,6 @@ const root = resolve(dirname(fileURLToPath(import.meta.url)), ".."); // peers already carry, so the TS6059 rootDir noise is excluded). const DEBT = { "@object-ui/plugin-form": { errors: 10, issue: 2919, note: "6x t() fallback-signature mismatch, 2x undefined index, 2x string|number" }, - "@object-ui/site": { errors: 7, issue: 2919, note: "TS2304 on Next's generated LayoutProps/PageProps; needs .next/types from a prior next build" }, "@object-ui/plugin-grid": { errors: 4, issue: 2919, note: "2x t() call signature + 2x TS2367 that are closure-mutation narrowing artifacts, NOT a logic bug" }, "@object-ui/cli": { errors: 4, issue: 2919, note: "tsup dts:true does not fail on these" }, "@object-ui/plugin-view": { errors: 3, issue: 2916, note: "Record missing the 'chart' key" }, @@ -45,6 +44,25 @@ const DEBT = { // package, the exemption dies, and the guard fails. const NOT_COMPILED = ["@object-ui/example-hello-world"]; +// Packages whose own `build` type-checks them, so a separate `type-check` script +// would only run the compiler twice. Unlike the `vite build` packages — which +// transpile without checking, the hole that caused #2911 — `next build` runs a +// full type-check unless `typescript.ignoreBuildErrors` is set. +// +// That escape hatch is exactly how this exemption could rot, so it is verified +// on every run rather than trusted: setting `ignoreBuildErrors` fails the guard. +const CHECKED_BY_OWN_BUILD = { + "@object-ui/site": { + build: "next build", + // Caveat worth knowing: the `docs` CI job runs this build only when + // `apps/site/` or `content/` changed (plus every push to main). A PR that + // only touches a workspace package in `transpilePackages` therefore does + // not re-check the site until it lands. Closing that would mean paying a + // Next build on many more PRs — a cost/coverage call, not a silent gap. + verifyNoIgnoreBuildErrors: "apps/site/next.config.mjs", + }, +}; + // ── Collect workspace packages ─────────────────────────────────────────────── const GROUPS = ["packages", "apps", "examples"]; @@ -77,6 +95,7 @@ function collect() { name: pkg.name, dir, hasScript: Boolean(pkg.scripts?.["type-check"]), + build: pkg.scripts?.build, hasBuild: Boolean(pkg.scripts?.build), hasTsconfig, }); @@ -93,7 +112,7 @@ const errors = []; // 1. Undeclared gap — a package born without a type-check script. for (const pkg of packages) { if (pkg.hasScript) continue; - if (DEBT[pkg.name] || NOT_COMPILED.includes(pkg.name)) continue; + if (DEBT[pkg.name] || NOT_COMPILED.includes(pkg.name) || CHECKED_BY_OWN_BUILD[pkg.name]) continue; errors.push( `${pkg.name} (${pkg.dir}) has no "type-check" script, so \`pnpm type-check\` skips it entirely.\n` + ` Add "type-check": "tsc --noEmit" to its package.json. If its types do not compile\n` + @@ -134,14 +153,58 @@ for (const name of NOT_COMPILED) { } } +// 4. Ratchet — "its own build checks it" only holds while that stays true. +for (const [name, spec] of Object.entries(CHECKED_BY_OWN_BUILD)) { + const pkg = byName.get(name); + if (!pkg) { + errors.push(`${name} is listed in CHECKED_BY_OWN_BUILD but is not a workspace package any more — delete the entry.`); + continue; + } + if (pkg.hasScript) { + errors.push(`${name} now has a "type-check" script — delete its CHECKED_BY_OWN_BUILD entry.`); + continue; + } + if (pkg.build !== spec.build) { + errors.push( + `${name} is exempt because its build is \`${spec.build}\`, which type-checks — but the build\n` + + ` script is now \`${pkg.build}\`. Re-confirm it still type-checks, then update or drop the entry.` + ); + continue; + } + // `next build` type-checks by default; `ignoreBuildErrors` silently disables + // it, which would turn this exemption into exactly the hole #2911 was about. + if (spec.verifyNoIgnoreBuildErrors) { + const configPath = resolve(root, spec.verifyNoIgnoreBuildErrors); + let config; + try { + config = readFileSync(configPath, "utf8"); + } catch { + errors.push( + `${name}: cannot read ${spec.verifyNoIgnoreBuildErrors}, so the exemption cannot be verified.\n` + + ` Point verifyNoIgnoreBuildErrors at the real config, or drop the exemption.` + ); + continue; + } + if (/ignoreBuildErrors\s*:\s*true/.test(config)) { + errors.push( + `${name} sets \`ignoreBuildErrors: true\` in ${spec.verifyNoIgnoreBuildErrors}, so \`${spec.build}\`\n` + + ` no longer type-checks it and nothing else does either. Remove that flag, or add a\n` + + ` "type-check" script and delete this exemption.` + ); + } + } +} + // ── Report ─────────────────────────────────────────────────────────────────── const checked = packages.filter((p) => p.hasScript).length; const debtCount = Object.keys(DEBT).length; const debtErrors = Object.values(DEBT).reduce((sum, d) => sum + d.errors, 0); +const byBuild = Object.keys(CHECKED_BY_OWN_BUILD).length; if (errors.length === 0) { console.log( - `✅ type-check coverage: ${checked}/${packages.length} packages checked, ` + + `✅ type-check coverage: ${checked}/${packages.length} via \`type-check\`, ` + + `${byBuild} via their own build, ` + `${debtCount} known-broken (${debtErrors} errors outstanding), ` + `${NOT_COMPILED.length} not compiled.` );