Skip to content

Commit 2473353

Browse files
authored
ci(superdoc): per-file checkJs gate for public-contract drift (SD-2863) (#3044)
* ci(superdoc): per-file checkJs gate for public-contract drift (SD-2863) The existing audit gates (declarations, consumer matrix, public-type assertion list) all check the published artifact. None of them catch the case where a public JSDoc typedef drifts from the implementation that consumes it — the IT-300 / IT-338 / SD-2514 class. Enabling project-wide `checkJs: true` cascades through `customConditions: ["source"]` resolution and surfaces ~6500 errors across 439 files, which is its own multi-PR ratchet rather than a gate. Lands the per-file pattern instead: each file in the gate adds `// @ts-check`, the gate script runs `tsc --noEmit` against the existing project tsconfig and filters output to the curated file list. A regression on a gated file fails CI; pre-existing errors elsewhere are ignored and tracked under follow-up tickets. First file under the gate is `packages/superdoc/src/helpers/ schema-introspection.js`. The probe immediately surfaced one real drift: the `extensions` parameter was typed `Array` (no type argument), which collapses to `Array<any>` for consumers — exactly the SD-2514 class. Replaced with `NonNullable<EditorOptions['extensions']>` so the parameter pulls its real shape from the public `EditorOptions` type. Adding more files later: add `// @ts-check` as the first line and append the path to `CHECKED_FILES` in `check-jsdoc.cjs`. Follow-up tickets cover the next files (use-find-replace, use-password-prompt, SuperDoc.js, etc.) one at a time. * ci(superdoc): fail check:jsdoc on tsc invocation errors (SD-2863) The gate scanned tsc output for diagnostics in CHECKED_FILES, but did not check whether tsc actually ran. Two silent-pass modes existed: - spawnSync sets result.error (e.g. ENOENT on a missing tsc binary, signal, permission denied). result.status is null, stdout/stderr are empty, and the script reports OK. - tsc exits non-zero with a structural error (missing tsconfig, internal crash) that does not match the (line,col) regex. allErrors is empty, so the gate reports OK on a build that never type-checked anything. Both now exit 1 with the captured failure context. * ci(superdoc): require @ts-check on gated files and fail on signal kill (SD-2863) Reviewer flagged two more silent-pass modes the previous patch missed: - A path can be in CHECKED_FILES without the file containing `// @ts-check`. Because the project tsconfig sets `checkJs: false`, TypeScript skips the file entirely, allErrors stays empty, and the gate reports OK even though the file has fully drifted. - spawnSync can return with `result.status === null` and `result.signal` set when tsc is killed by a signal (SIGKILL/OOM/SIGTERM) mid-run. If the partial output happens to contain parseable diagnostics that match no gated file, the structural-failure branch does not fire. Both fail explicitly now. Pre-flight refuses to run tsc unless every gated file exists and contains `// @ts-check` in its first 4 KiB; the post-spawn check fails on `result.signal !== null`. Verified locally by deleting the directive and by killing a fake tsc with SIGKILL.
1 parent 7eb7412 commit 2473353

4 files changed

Lines changed: 171 additions & 1 deletion

File tree

.github/workflows/ci-superdoc.yml

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -113,6 +113,13 @@ jobs:
113113
- name: Typecheck
114114
run: pnpm run type-check
115115

116+
- name: Public-contract checkJs (SD-2863)
117+
# Gated subset of public-contract files run with `// @ts-check`. The
118+
# script greps tsc output for errors in the curated file list and
119+
# ignores the wider 1500+ errors from the broader super-editor source
120+
# tree (those are tracked under SD-2863 follow-up tickets).
121+
run: pnpm --filter superdoc run check:jsdoc
122+
116123
- name: Consumer typecheck (matrix)
117124
# The matrix script owns the published-package validation path:
118125
# it packs superdoc, installs the tarball into the standalone

packages/superdoc/package.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -112,6 +112,7 @@
112112
"postbuild": "node ./scripts/ensure-types.cjs && node ./scripts/audit-bundle.cjs && node ./scripts/audit-declarations.cjs",
113113
"audit:declarations": "node ./scripts/audit-declarations.cjs",
114114
"audit:declarations:informational": "node ./scripts/audit-declarations.cjs --informational",
115+
"check:jsdoc": "node ./scripts/check-jsdoc.cjs",
115116
"build:es": "vite build && pnpm run postbuild",
116117
"watch:es": "vite build --watch",
117118
"build:cdn": "vite build --config vite.config.cdn.js",
Lines changed: 161 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,161 @@
1+
#!/usr/bin/env node
2+
/**
3+
* SD-2863: per-file checkJs gate for the public-contract surface.
4+
*
5+
* Why this exists in this shape (and not as a plain `tsc -p tsconfig.checkjs.json`):
6+
*
7+
* The codebase uses `customConditions: ["source"]`, which makes TypeScript
8+
* resolve `import { Editor } from '@superdoc/super-editor'` to the source
9+
* `.js`/`.ts` files of the workspace package. With `// @ts-check` enabled on
10+
* any file in this package, TS follows those imports and type-checks the
11+
* super-editor source too — about 6500 errors. Those errors are real (they
12+
* are the broader SD-2863 work) but they are not what this PR is trying to
13+
* gate. The gate here is "files in CHECKED_FILES must stay clean."
14+
*
15+
* The script:
16+
*
17+
* 1. Runs `tsc --noEmit -p packages/superdoc/tsconfig.json`. Because each
18+
* file in CHECKED_FILES has `// @ts-check`, TS reports errors on those
19+
* files even though the project-wide `checkJs` is `false`.
20+
* 2. Filters the tsc output to errors whose path matches an entry in
21+
* CHECKED_FILES.
22+
* 3. Exits non-zero if any matched the filter; exits zero if not.
23+
*
24+
* Adding a new file to the gate:
25+
*
26+
* 1. Add `// @ts-check` as the first line of the file.
27+
* 2. Add the file's path (relative to `packages/superdoc/`) to
28+
* CHECKED_FILES below.
29+
* 3. Run `node packages/superdoc/scripts/check-jsdoc.cjs` and fix what
30+
* surfaces.
31+
*
32+
* The intent is for CHECKED_FILES to grow over time as the team ratchets
33+
* checkJs across the public-contract surface. SD-2863 lands the pattern;
34+
* follow-up tickets land the additional files.
35+
*/
36+
37+
const fs = require('fs');
38+
const path = require('path');
39+
const { spawnSync } = require('child_process');
40+
41+
const CHECKED_FILES = [
42+
'src/helpers/schema-introspection.js',
43+
];
44+
45+
const packageDir = path.resolve(__dirname, '..');
46+
const repoRoot = path.resolve(packageDir, '..', '..');
47+
48+
const tscBin = path.join(repoRoot, 'node_modules', '.bin', 'tsc');
49+
const tsconfigPath = path.join(packageDir, 'tsconfig.json');
50+
51+
// Pre-flight: every file in CHECKED_FILES must opt into `// @ts-check`.
52+
// The project's tsconfig sets `checkJs: false`, so a JS file without the
53+
// directive is not type-checked at all. Without this guard, removing or
54+
// forgetting the directive on a listed file makes the gate silently stop
55+
// covering it — the script keeps reporting OK even though the file has
56+
// drifted.
57+
const missingDirective = [];
58+
const missingFiles = [];
59+
for (const rel of CHECKED_FILES) {
60+
const abs = path.join(packageDir, rel);
61+
if (!fs.existsSync(abs)) {
62+
missingFiles.push(rel);
63+
continue;
64+
}
65+
// The directive only takes effect when it precedes any non-comment
66+
// statement, so it lives near the top. 4 KiB is plenty of margin for
67+
// a leading license/doc block.
68+
const head = fs.readFileSync(abs, 'utf8').slice(0, 4096);
69+
if (!/^\s*\/\/\s*@ts-check\b/m.test(head)) {
70+
missingDirective.push(rel);
71+
}
72+
}
73+
74+
if (missingFiles.length > 0) {
75+
console.error('[check-jsdoc] gated files do not exist:');
76+
for (const f of missingFiles) console.error(` - ${f}`);
77+
process.exit(1);
78+
}
79+
if (missingDirective.length > 0) {
80+
console.error('[check-jsdoc] gated files are missing the `// @ts-check` directive:');
81+
for (const f of missingDirective) console.error(` - ${f}`);
82+
console.error('Each gated file must opt into checkJs explicitly.');
83+
console.error('Add `// @ts-check` as the first non-blank line, then re-run.');
84+
process.exit(1);
85+
}
86+
87+
const result = spawnSync(tscBin, ['--noEmit', '-p', tsconfigPath], {
88+
encoding: 'utf8',
89+
cwd: repoRoot,
90+
});
91+
92+
// Fail fast if tsc itself could not be spawned (ENOENT on the binary,
93+
// EACCES, etc.). Without this guard, a missing `tsc` leaves
94+
// `result.error` set, empty stdout/stderr, and the rest of the script
95+
// would happily report "OK" because it found zero parseable errors.
96+
if (result.error) {
97+
console.error(`[check-jsdoc] failed to invoke tsc at ${tscBin}: ${result.error.message}`);
98+
process.exit(1);
99+
}
100+
101+
// Killed by a signal (SIGKILL/OOM/SIGTERM) mid-run. spawnSync sets
102+
// `result.status` to null in that case and may leave partial output
103+
// containing parseable diagnostics, which would otherwise sneak past
104+
// the structural-failure check below.
105+
if (result.signal !== null) {
106+
console.error(`[check-jsdoc] tsc was killed by signal: ${result.signal}`);
107+
process.exit(1);
108+
}
109+
110+
const output = `${result.stdout || ''}${result.stderr || ''}`;
111+
112+
// Match each `path/to/file(line,col): error TSxxxx: ...` row. tsc emits
113+
// paths relative to the cwd we ran from (repoRoot).
114+
const allErrors = output
115+
.split('\n')
116+
.filter((line) => /\.[jt]sx?\(\d+,\d+\):\s+error\s+TS\d+:/.test(line));
117+
118+
// Catch the structural-failure mode: tsc exited non-zero but produced no
119+
// parseable diagnostics. That means the failure is something like a
120+
// missing tsconfig, an internal compiler crash, or a config error,
121+
// rather than a normal type-check fail; the gate cannot reason about it.
122+
if (result.status !== 0 && allErrors.length === 0) {
123+
console.error('[check-jsdoc] tsc exited with a non-zero status but produced no parseable diagnostics.');
124+
console.error(`Status: ${result.status}`);
125+
console.error(`Output:\n${output || '(empty)'}`);
126+
process.exit(1);
127+
}
128+
129+
const checkedAbsolute = CHECKED_FILES.map((rel) => path.join(packageDir, rel));
130+
131+
const isCheckedError = (line) => {
132+
const match = line.match(/^([^(]+)\(\d+,\d+\):/);
133+
if (!match) return false;
134+
const filePath = path.resolve(repoRoot, match[1]);
135+
return checkedAbsolute.includes(filePath);
136+
};
137+
138+
const checkedErrors = allErrors.filter(isCheckedError);
139+
140+
console.log('[check-jsdoc] SD-2863 public-contract checkJs gate');
141+
console.log('='.repeat(72));
142+
console.log(`Files under gate: ${CHECKED_FILES.length}`);
143+
for (const f of CHECKED_FILES) {
144+
console.log(` - ${f}`);
145+
}
146+
console.log();
147+
148+
if (checkedErrors.length === 0) {
149+
console.log(`OK ${CHECKED_FILES.length} gated file${CHECKED_FILES.length === 1 ? '' : 's'} clean.`);
150+
console.log(` (${allErrors.length} non-gated error${allErrors.length === 1 ? '' : 's'} in the wider tsc run, ignored — see SD-2863 follow-up tickets.)`);
151+
process.exit(0);
152+
}
153+
154+
console.log(`FAIL ${checkedErrors.length} error${checkedErrors.length === 1 ? '' : 's'} in gated files:`);
155+
for (const line of checkedErrors) {
156+
console.log(` ${line}`);
157+
}
158+
console.log();
159+
console.log('Each error means a public-contract JSDoc has drifted from the implementation.');
160+
console.log('Fix the type or the code so they match. Adding `// @ts-ignore` is not the answer.');
161+
process.exit(1);

packages/superdoc/src/helpers/schema-introspection.js

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,10 @@
1+
// @ts-check
12
import { Editor, getRichTextExtensions, getStarterExtensions } from '@superdoc/super-editor';
23

34
/**
45
* @typedef {Object} SchemaIntrospectionOptions
56
* @property {import('@superdoc/super-editor').Editor} [editor] - Existing Editor instance to introspect.
6-
* @property {Array} [extensions] - Extension list to build a schema from.
7+
* @property {NonNullable<import('@superdoc/super-editor').EditorOptions['extensions']>} [extensions] - Extension list to build a schema from.
78
* @property {'docx' | 'text' | 'html'} [mode] - Editor mode when building a schema. Defaults to 'docx'.
89
*/
910

0 commit comments

Comments
 (0)