Skip to content

Commit 40225e9

Browse files
yinlianghuiclaude
andauthored
fix(build): fail the build when tsup emits no declarations, so a DTS-less dist cannot be cached as a full build (#12076)
* fix(build): fail the build when tsup emits no declarations, so a DTS-less dist cannot be cached as a full build tsup runs DTS generation in a worker thread and settles its promise only from that worker's `message` events -- it registers no `error` and no `exit` handler. A worker that dies without posting a message leaves the promise unsettled; the event loop drains and node exits 0 with the JS pass already written. turbo caches successful tasks, so the DTS-less dist lands in the cache under the hash of an ordinary full build and every later run replays it. Assert the declarations the manifest promises before the build may succeed. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UjM2ia8Av1v5NqfqQEQmC6 * fix(build): put the dispatch behind isEntrypoint so importing the guard cannot exit the importer check:entry-guard: a scripts/** file that exports bindings must not run on import. Fitting for this one -- an unguarded process.exit here would end an importer mid-import with status 0, the same 'exit 0 reads as success' failure the guard itself exists to catch. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UjM2ia8Av1v5NqfqQEQmC6 --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 402113d commit 40225e9

2 files changed

Lines changed: 267 additions & 1 deletion

File tree

packages/plugins/plugin-auth/package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@
1919
}
2020
},
2121
"scripts": {
22-
"build": "tsup",
22+
"build": "tsup && node ../../../scripts/check-dts-emitted.mjs",
2323
"test": "vitest run",
2424
"typecheck": "tsc --noEmit && tsc --noEmit -p tsconfig.examples.json"
2525
},

scripts/check-dts-emitted.mjs

Lines changed: 266 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,266 @@
1+
#!/usr/bin/env node
2+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
3+
//
4+
// check-dts-emitted -- run as the LAST step of a package's own build: every
5+
// declaration file the package's manifest promises must actually be on disk
6+
// before the build is allowed to report success.
7+
//
8+
// node ../../../scripts/check-dts-emitted.mjs # from the package directory
9+
// node scripts/check-dts-emitted.mjs --self-test
10+
//
11+
// ---------------------------------------------------------------------------
12+
// THE DEFECT IT EXISTS TO CLOSE (#11907)
13+
//
14+
// tsup 8.5.1 runs DTS generation in a `worker_threads.Worker` (dist/rollup.js)
15+
// and settles the build promise ONLY from that worker's `message` events:
16+
//
17+
// await new Promise((resolve, reject) => {
18+
// const worker = new Worker(path.join(__dirname, './rollup.js'));
19+
// worker.postMessage({ ... });
20+
// worker.on('message', (data) => {
21+
// if (data === 'error') { terminateWorker(); reject(...); }
22+
// else if (data === 'success') { terminateWorker(); resolve(); }
23+
// ...
24+
// });
25+
// // <- no worker.on('error'), no worker.on('exit')
26+
// });
27+
//
28+
// There is no `error` handler and no `exit` handler. If the worker dies without
29+
// posting a message -- OOM under memory pressure, a hard `process.exit`, a
30+
// terminated thread -- neither branch ever runs, the promise NEVER SETTLES, the
31+
// event loop drains, and **Node exits 0**. tsup's `Promise.all([dtsTask(),
32+
// mainTasks()])` never resolves either, so nothing prints and nothing throws:
33+
// the JS pass has already written `dist/`, so the run leaves a dist with
34+
// `index.js` / `index.mjs` / maps and ZERO `.d.ts` files, and reports success.
35+
//
36+
// That is a correctness problem for the build CACHE, not just for one build.
37+
// `OS_SKIP_DTS` is declared in turbo.json `globalEnv` and it works: measured on
38+
// this package, `@objectstack/plugin-auth#build` hashes `ce8fa0947ab2f8f8` with
39+
// the variable unset and `a6411042c68db782` with `OS_SKIP_DTS=1`. So a
40+
// deliberate skip-DTS build is already cache-isolated from a normal one. The
41+
// silent worker death is NOT: it happens on a run with `OS_SKIP_DTS` unset,
42+
// which hashes as -- because it *is* -- an ordinary full build. turbo sees exit
43+
// 0 and caches the DTS-less `dist/**` under the ordinary hash. Every later run
44+
// in that worktree replays it, so the AGENTS.md section 9 remedy (`pnpm build`)
45+
// does not clear it: a plain rebuild is a cache HIT that restores the same bad
46+
// artifact. Only `--force` (or deleting the entry) replaces it.
47+
//
48+
// The package's own typecheck is then what reds, on a diff that never touched
49+
// it -- `error TS7016: Could not find a declaration file for module '...'` --
50+
// which reads as "your change broke this package".
51+
//
52+
// So the repair is at the cause: a build that was supposed to emit declarations
53+
// and did not must EXIT NON-ZERO. turbo only caches successful tasks, so a
54+
// failing build never becomes a cache entry, and the fault cannot outlive the
55+
// run that produced it. Nothing here loosens a hash or busts a cache.
56+
// ---------------------------------------------------------------------------
57+
58+
import { readFileSync, statSync } from 'node:fs';
59+
import { resolve, relative } from 'node:path';
60+
61+
import { isEntrypoint } from './invoked-as.mjs';
62+
63+
/**
64+
* Every declaration path the manifest promises a consumer.
65+
*
66+
* Deliberately the DECLARED surface (`types`, `typings`, and `types`
67+
* conditions inside `exports`), not "whatever tsup might have emitted": these
68+
* are the paths a consumer's resolver actually reads, so a missing one is
69+
* exactly the breakage TS7016 reports. Emitting extra declarations that the
70+
* manifest never names is not this guard's business.
71+
*/
72+
export function declaredDeclarationPaths(manifest) {
73+
const paths = new Set();
74+
// Normalise BEFORE the Set: `types` is conventionally written bare
75+
// (`dist/index.d.ts`) while an `exports` condition must be `./`-relative
76+
// (`./dist/index.d.ts`). Deduping after the strip reports the same file twice.
77+
const add = (p) => paths.add(p.replace(/^\.\//, ''));
78+
79+
for (const key of ['types', 'typings']) {
80+
if (typeof manifest[key] === 'string') add(manifest[key]);
81+
}
82+
83+
// Inside `exports`, a declaration target can appear under a `types`
84+
// condition at any depth. Match on the extension rather than the key so a
85+
// nested/array form cannot slip past.
86+
const collect = (node) => {
87+
if (typeof node === 'string') {
88+
// Only `./`-relative targets are paths; a bare specifier is a re-export.
89+
if (node.startsWith('./') && /\.d\.[cm]?ts$/.test(node)) add(node);
90+
return;
91+
}
92+
if (Array.isArray(node)) {
93+
for (const v of node) collect(v);
94+
return;
95+
}
96+
if (node && typeof node === 'object') for (const v of Object.values(node)) collect(v);
97+
};
98+
collect(manifest.exports);
99+
100+
return [...paths].sort();
101+
}
102+
103+
/** Missing or empty declaration files, given a resolver for file size. */
104+
export function missingDeclarations(declared, sizeOf) {
105+
const missing = [];
106+
for (const rel of declared) {
107+
const size = sizeOf(rel);
108+
if (size === null) missing.push({ path: rel, why: 'missing' });
109+
else if (size === 0) missing.push({ path: rel, why: 'empty' });
110+
}
111+
return missing;
112+
}
113+
114+
function sizeOnDisk(dir) {
115+
return (rel) => {
116+
try {
117+
const s = statSync(resolve(dir, rel));
118+
return s.isFile() ? s.size : null;
119+
} catch {
120+
return null;
121+
}
122+
};
123+
}
124+
125+
function run(dir) {
126+
// `OS_SKIP_DTS` set means the declarations are absent ON PURPOSE. That run
127+
// hashes differently from a full build (globalEnv, verified above), so its
128+
// artifact cannot be served to a run that wants declarations -- there is
129+
// nothing for this guard to protect.
130+
if (process.env.OS_SKIP_DTS) {
131+
console.log('check-dts-emitted: OS_SKIP_DTS is set - declarations skipped by request, not checked.');
132+
return 0;
133+
}
134+
135+
let manifest;
136+
const manifestPath = resolve(dir, 'package.json');
137+
try {
138+
manifest = JSON.parse(readFileSync(manifestPath, 'utf8'));
139+
} catch (err) {
140+
console.error(`\nx check-dts-emitted: cannot read ${manifestPath}: ${err.message}`);
141+
console.error(' This runs from the package directory, as the last step of that package\'s build.\n');
142+
return 1;
143+
}
144+
145+
const declared = declaredDeclarationPaths(manifest);
146+
if (declared.length === 0) {
147+
console.log(`check-dts-emitted: ${manifest.name ?? dir} declares no declaration entry points - nothing to check.`);
148+
return 0;
149+
}
150+
151+
const missing = missingDeclarations(declared, sizeOnDisk(dir));
152+
if (missing.length === 0) {
153+
console.log(
154+
`check-dts-emitted: ${manifest.name ?? dir} - ${declared.length}/${declared.length} declared declaration file(s) present.`,
155+
);
156+
return 0;
157+
}
158+
159+
const rel = relative(process.cwd(), dir) || '.';
160+
console.error(`\nx ${manifest.name ?? rel}: the build finished but did NOT emit the declarations this package promises.\n`);
161+
for (const m of missing) console.error(` ${m.why.padEnd(7)} ${m.path}`);
162+
console.error(
163+
'\n package.json points consumers at these paths, so without them any dependent\n' +
164+
" typecheck fails with TS7016 \"Could not find a declaration file for module\n" +
165+
` '${manifest.name ?? ''}'\" - and it reads as though THEIR change broke this package.\n` +
166+
'\n Most likely cause (#11907): tsup runs DTS generation in a worker thread and\n' +
167+
" settles its promise only on the worker's `message` events - it registers no\n" +
168+
' `error` and no `exit` handler. A worker that dies (OOM under memory pressure\n' +
169+
' is the observed one) posts neither "success" nor "error", so the promise never\n' +
170+
' settles, the event loop drains, and node exits 0 with the JS already written.\n' +
171+
'\n This guard is what stops that exit 0 from becoming a CACHED artifact: turbo\n' +
172+
' caches only successful tasks, and a skip-DTS run already hashes differently,\n' +
173+
' so failing here keeps a DTS-less dist from ever being served as a full build.\n' +
174+
'\n Re-run the build. If it keeps failing here, build with more headroom:\n' +
175+
' NODE_OPTIONS=--max-old-space-size=8192 pnpm --filter <pkg> build\n' +
176+
'\n If a poisoned entry was cached before this guard existed, a plain rebuild is a\n' +
177+
' cache HIT that restores it - clear it with:\n' +
178+
' pnpm exec turbo run build --filter <pkg> --force\n',
179+
);
180+
return 1;
181+
}
182+
183+
// --- self-test ------------------------------------------------------------
184+
// The two directions this guard can be wrong in are both silent: over-matching
185+
// makes every build fail, under-matching waves the DTS-less dist through -- the
186+
// exact artifact #11907 recorded. So both get asserted rather than assumed.
187+
function selfTest() {
188+
const failures = [];
189+
const eq = (label, actual, expected) => {
190+
const a = JSON.stringify(actual);
191+
const e = JSON.stringify(expected);
192+
if (a !== e) failures.push(`${label}\n expected ${e}\n actual ${a}`);
193+
};
194+
195+
const authLike = {
196+
name: '@objectstack/plugin-auth',
197+
types: 'dist/index.d.ts',
198+
exports: {
199+
'.': {
200+
types: './dist/index.d.ts',
201+
import: './dist/index.mjs',
202+
require: './dist/index.js',
203+
},
204+
'./rate-limit-storage': {
205+
types: './dist/rate-limit-storage.d.ts',
206+
import: './dist/rate-limit-storage.mjs',
207+
require: './dist/rate-limit-storage.js',
208+
},
209+
},
210+
};
211+
eq('collects every declared declaration, deduped across `types` and `exports`', declaredDeclarationPaths(authLike), [
212+
'dist/index.d.ts',
213+
'dist/rate-limit-storage.d.ts',
214+
]);
215+
216+
eq(
217+
'ignores JS entry points - only declarations are this guard\'s business',
218+
declaredDeclarationPaths({ main: 'dist/index.js', module: 'dist/index.mjs', exports: { '.': './dist/index.mjs' } }),
219+
[],
220+
);
221+
222+
eq('ignores bare re-export specifiers', declaredDeclarationPaths({ exports: { '.': 'other-pkg/types' } }), []);
223+
224+
eq('finds .d.mts and .d.cts', declaredDeclarationPaths({ exports: { '.': { types: './dist/index.d.mts' } } }), [
225+
'dist/index.d.mts',
226+
]);
227+
228+
eq('handles the array form inside exports', declaredDeclarationPaths({ exports: { '.': [{ types: './dist/a.d.ts' }] } }), [
229+
'dist/a.d.ts',
230+
]);
231+
232+
// The load-bearing direction: the #11907 artifact must be caught.
233+
const declared = declaredDeclarationPaths(authLike);
234+
const dtsLessDist = (rel) => (rel.endsWith('.d.ts') ? null : 100);
235+
eq(
236+
'REJECTS the #11907 artifact: JS present, zero declarations',
237+
missingDeclarations(declared, dtsLessDist).map((m) => `${m.why}:${m.path}`),
238+
['missing:dist/index.d.ts', 'missing:dist/rate-limit-storage.d.ts'],
239+
);
240+
241+
eq('ACCEPTS a healthy dist', missingDeclarations(declared, () => 100), []);
242+
243+
eq(
244+
'rejects a zero-byte declaration - present but useless',
245+
missingDeclarations(declared, (rel) => (rel === 'dist/index.d.ts' ? 0 : 100)).map((m) => `${m.why}:${m.path}`),
246+
['empty:dist/index.d.ts'],
247+
);
248+
249+
if (failures.length > 0) {
250+
console.error(`\nx check-dts-emitted self-test: ${failures.length} failure(s)\n`);
251+
for (const f of failures) console.error(` - ${f}\n`);
252+
return 1;
253+
}
254+
console.log('check-dts-emitted self-test: all assertions passed.');
255+
return 0;
256+
}
257+
258+
// Behind the entrypoint guard: this module exports its two predicates so they
259+
// can be unit-tested and reused, and an unguarded `process.exit` here would end
260+
// any importer mid-import -- with status 0 on the healthy path, so the importer
261+
// would read it as success. That is the same "exit 0 means nothing went wrong"
262+
// failure this guard exists to catch, one level up.
263+
if (isEntrypoint(import.meta.url)) {
264+
const isSelfTest = process.argv.includes('--self-test');
265+
process.exit(isSelfTest ? selfTest() : run(process.cwd()));
266+
}

0 commit comments

Comments
 (0)