Skip to content

Commit b5efe85

Browse files
os-zhuangclaude
andauthored
chore(type-check): site is not debt — its own next build type-checks it (#2924)
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: Jack Zhuang <277994282+os-zhuang@users.noreply.github.com> Co-authored-by: Claude <noreply@anthropic.com>
1 parent b39f4b3 commit b5efe85

1 file changed

Lines changed: 66 additions & 3 deletions

File tree

scripts/check-type-check-coverage.mjs

Lines changed: 66 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,6 @@ const root = resolve(dirname(fileURLToPath(import.meta.url)), "..");
3030
// peers already carry, so the TS6059 rootDir noise is excluded).
3131
const DEBT = {
3232
"@object-ui/plugin-form": { errors: 10, issue: 2919, note: "6x t() fallback-signature mismatch, 2x undefined index, 2x string|number" },
33-
"@object-ui/site": { errors: 7, issue: 2919, note: "TS2304 on Next's generated LayoutProps/PageProps; needs .next/types from a prior next build" },
3433
"@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" },
3534
"@object-ui/cli": { errors: 4, issue: 2919, note: "tsup dts:true does not fail on these" },
3635
"@object-ui/plugin-view": { errors: 3, issue: 2916, note: "Record<ViewType,...> missing the 'chart' key" },
@@ -45,6 +44,25 @@ const DEBT = {
4544
// package, the exemption dies, and the guard fails.
4645
const NOT_COMPILED = ["@object-ui/example-hello-world"];
4746

47+
// Packages whose own `build` type-checks them, so a separate `type-check` script
48+
// would only run the compiler twice. Unlike the `vite build` packages — which
49+
// transpile without checking, the hole that caused #2911 — `next build` runs a
50+
// full type-check unless `typescript.ignoreBuildErrors` is set.
51+
//
52+
// That escape hatch is exactly how this exemption could rot, so it is verified
53+
// on every run rather than trusted: setting `ignoreBuildErrors` fails the guard.
54+
const CHECKED_BY_OWN_BUILD = {
55+
"@object-ui/site": {
56+
build: "next build",
57+
// Caveat worth knowing: the `docs` CI job runs this build only when
58+
// `apps/site/` or `content/` changed (plus every push to main). A PR that
59+
// only touches a workspace package in `transpilePackages` therefore does
60+
// not re-check the site until it lands. Closing that would mean paying a
61+
// Next build on many more PRs — a cost/coverage call, not a silent gap.
62+
verifyNoIgnoreBuildErrors: "apps/site/next.config.mjs",
63+
},
64+
};
65+
4866
// ── Collect workspace packages ───────────────────────────────────────────────
4967
const GROUPS = ["packages", "apps", "examples"];
5068

@@ -77,6 +95,7 @@ function collect() {
7795
name: pkg.name,
7896
dir,
7997
hasScript: Boolean(pkg.scripts?.["type-check"]),
98+
build: pkg.scripts?.build,
8099
hasBuild: Boolean(pkg.scripts?.build),
81100
hasTsconfig,
82101
});
@@ -93,7 +112,7 @@ const errors = [];
93112
// 1. Undeclared gap — a package born without a type-check script.
94113
for (const pkg of packages) {
95114
if (pkg.hasScript) continue;
96-
if (DEBT[pkg.name] || NOT_COMPILED.includes(pkg.name)) continue;
115+
if (DEBT[pkg.name] || NOT_COMPILED.includes(pkg.name) || CHECKED_BY_OWN_BUILD[pkg.name]) continue;
97116
errors.push(
98117
`${pkg.name} (${pkg.dir}) has no "type-check" script, so \`pnpm type-check\` skips it entirely.\n` +
99118
` 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) {
134153
}
135154
}
136155

156+
// 4. Ratchet — "its own build checks it" only holds while that stays true.
157+
for (const [name, spec] of Object.entries(CHECKED_BY_OWN_BUILD)) {
158+
const pkg = byName.get(name);
159+
if (!pkg) {
160+
errors.push(`${name} is listed in CHECKED_BY_OWN_BUILD but is not a workspace package any more — delete the entry.`);
161+
continue;
162+
}
163+
if (pkg.hasScript) {
164+
errors.push(`${name} now has a "type-check" script — delete its CHECKED_BY_OWN_BUILD entry.`);
165+
continue;
166+
}
167+
if (pkg.build !== spec.build) {
168+
errors.push(
169+
`${name} is exempt because its build is \`${spec.build}\`, which type-checks — but the build\n` +
170+
` script is now \`${pkg.build}\`. Re-confirm it still type-checks, then update or drop the entry.`
171+
);
172+
continue;
173+
}
174+
// `next build` type-checks by default; `ignoreBuildErrors` silently disables
175+
// it, which would turn this exemption into exactly the hole #2911 was about.
176+
if (spec.verifyNoIgnoreBuildErrors) {
177+
const configPath = resolve(root, spec.verifyNoIgnoreBuildErrors);
178+
let config;
179+
try {
180+
config = readFileSync(configPath, "utf8");
181+
} catch {
182+
errors.push(
183+
`${name}: cannot read ${spec.verifyNoIgnoreBuildErrors}, so the exemption cannot be verified.\n` +
184+
` Point verifyNoIgnoreBuildErrors at the real config, or drop the exemption.`
185+
);
186+
continue;
187+
}
188+
if (/ignoreBuildErrors\s*:\s*true/.test(config)) {
189+
errors.push(
190+
`${name} sets \`ignoreBuildErrors: true\` in ${spec.verifyNoIgnoreBuildErrors}, so \`${spec.build}\`\n` +
191+
` no longer type-checks it and nothing else does either. Remove that flag, or add a\n` +
192+
` "type-check" script and delete this exemption.`
193+
);
194+
}
195+
}
196+
}
197+
137198
// ── Report ───────────────────────────────────────────────────────────────────
138199
const checked = packages.filter((p) => p.hasScript).length;
139200
const debtCount = Object.keys(DEBT).length;
140201
const debtErrors = Object.values(DEBT).reduce((sum, d) => sum + d.errors, 0);
202+
const byBuild = Object.keys(CHECKED_BY_OWN_BUILD).length;
141203

142204
if (errors.length === 0) {
143205
console.log(
144-
`✅ type-check coverage: ${checked}/${packages.length} packages checked, ` +
206+
`✅ type-check coverage: ${checked}/${packages.length} via \`type-check\`, ` +
207+
`${byBuild} via their own build, ` +
145208
`${debtCount} known-broken (${debtErrors} errors outstanding), ` +
146209
`${NOT_COMPILED.length} not compiled.`
147210
);

0 commit comments

Comments
 (0)