fix(build): make dts type errors fail the build instead of exiting 0

.changeset/dts-build-exit-code-5370.md

@@ -0,0 +1,7 @@
+---
+---
+
+Build tooling only — this publishes nothing, declared explicitly with an empty frontmatter
+rather than left undeclared. No package `src/` is touched: `@object-ui/layout`'s `vite build`
+now exits non-zero when the declaration step reports type errors, instead of printing them
+and exiting 0 (objectui#5370). The typings and the JavaScript it emits are unchanged.

packages/layout/vite.config.ts

@@ -4,6 +4,7 @@ import dts from 'vite-plugin-dts';
 import { resolve } from 'path';
 
 import { createDtsExplicitExtensions } from '../../scripts/vite-dts-explicit-extensions.ts';
+import { createDtsFailOnTypeErrors } from '../../scripts/vite-dts-fail-on-type-errors.ts';
 
 export default defineConfig({
   plugins: [
@@ -27,6 +28,13 @@ export default defineConfig({
       // verdict about specifier-preserving `.js` builds — correctly never
       // scanned this package. See the module header for the full argument.
       ...createDtsExplicitExtensions({ packageDir: __dirname }),
+      // A type error the declaration program already found and printed used to
+      // leave `vite build` exiting 0 — objectui#5370. This makes it fatal.
+      // Disjoint hooks from the factory above (`afterDiagnostic` vs
+      // `beforeWriteFile`/`afterBuild`), so the two spreads cannot overwrite
+      // each other; a future factory that overlaps must be composed, not
+      // spread, because a duplicate key silently keeps the last one.
+      ...createDtsFailOnTypeErrors({ packageDir: __dirname }),
     }),
   ],
   build: {

scripts/__tests__/vite-dts-fail-on-type-errors.test.ts

@@ -0,0 +1,146 @@
+import fs from 'node:fs';
+import os from 'node:os';
+import path from 'node:path';
+
+import ts from 'typescript';
+import { describe, expect, it } from 'vitest';
+
+import { createDtsExplicitExtensions } from '../vite-dts-explicit-extensions';
+import {
+  createDtsFailOnTypeErrors,
+  describeFatalDiagnostics,
+  formatDiagnostic,
+  selectFatalDiagnostics,
+} from '../vite-dts-fail-on-type-errors';
+
+/**
+ * objectui#5370 — `vite-plugin-dts` computed a type program's diagnostics,
+ * PRINTED them, and let `vite build` exit 0 anyway. Measured on
+ * `@object-ui/layout` at `478ec54ce` with one deliberate TS2353: the error in
+ * the log, `BUILD_EXIT=0` underneath it.
+ *
+ * What is pinned here, and why each case is not covered by the next:
+ *
+ *   1. **A real TypeScript diagnostic is fatal.** The subject is fed
+ *      diagnostics from an actual `ts.createProgram` over a fixture, not from
+ *      hand-rolled objects, so the predicate is pinned against the shape the
+ *      plugin really hands it.
+ *   2. **Only errors are fatal.** Warning / suggestion / message diagnostics
+ *      pass. This is invisible in a green build and is the difference between
+ *      a gate that survives and a gate someone switches off — which would take
+ *      the errors with it.
+ *   3. **The message carries the diagnostics.** The plugin logs them once, at
+ *      program creation, hundreds of lines above the failure; a `| tail` is
+ *      what hid them in the first place. A failure that only says "there were
+ *      errors" reproduces the defect one layer up.
+ *   4. **The hooks are disjoint from the other dts factory.**
+ *      `packages/layout/vite.config.ts` spreads both into one object literal,
+ *      where a duplicate key silently keeps the last one and reports nothing.
+ */
+
+/** Diagnostics from a real compile of one throwaway file. */
+function diagnosticsFor(source: string): { diagnostics: readonly ts.Diagnostic[]; dir: string } {
+  const dir = fs.mkdtempSync(path.join(fs.realpathSync(os.tmpdir()), 'dts-fail-'));
+  const file = path.join(dir, 'probe.ts');
+  fs.writeFileSync(file, source);
+
+  const program = ts.createProgram([file], {
+    strict: true,
+    noEmit: true,
+    target: ts.ScriptTarget.ES2022,
+    moduleResolution: ts.ModuleResolutionKind.Bundler,
+    module: ts.ModuleKind.ESNext,
+  });
+
+  return {
+    diagnostics: [...program.getSemanticDiagnostics(), ...program.getSyntacticDiagnostics()],
+    dir: dir.replace(/\\/g, '/'),
+  };
+}
+
+/** A diagnostic of a given category, with no file attached. */
+function bare(category: ts.DiagnosticCategory, code: number, messageText: string): ts.Diagnostic {
+  return { category, code, messageText, file: undefined, start: undefined, length: undefined };
+}
+
+describe('createDtsFailOnTypeErrors', () => {
+  it('throws on a real type error the declaration program would have printed and swallowed', () => {
+    const { diagnostics, dir } = diagnosticsFor("export const answer: number = 'not a number';\n");
+    expect(diagnostics.length).toBeGreaterThan(0);
+
+    const hooks = createDtsFailOnTypeErrors({ packageDir: dir });
+
+    expect(() => hooks.afterDiagnostic(diagnostics)).toThrowError(/TS2322/);
+    expect(() => hooks.afterDiagnostic(diagnostics)).toThrowError(/must not exit 0/);
+  });
+
+  it('stays silent on a clean program — the green build is unchanged', () => {
+    const { diagnostics, dir } = diagnosticsFor("export const answer: number = 42;\n");
+    expect(diagnostics).toHaveLength(0);
+
+    const hooks = createDtsFailOnTypeErrors({ packageDir: dir });
+    expect(() => hooks.afterDiagnostic(diagnostics)).not.toThrow();
+  });
+
+  it('fails on errors only — warnings, suggestions and messages are not build failures', () => {
+    const noisy = [
+      bare(ts.DiagnosticCategory.Warning, 6133, "'x' is declared but never used."),
+      bare(ts.DiagnosticCategory.Suggestion, 80001, 'File is a CommonJS module.'),
+      bare(ts.DiagnosticCategory.Message, 6194, 'Found 0 errors.'),
+    ];
+    const hooks = createDtsFailOnTypeErrors({ packageDir: '/pkg' });
+
+    expect(selectFatalDiagnostics(noisy)).toHaveLength(0);
+    expect(() => hooks.afterDiagnostic(noisy)).not.toThrow();
+
+    const withError = [...noisy, bare(ts.DiagnosticCategory.Error, 2353, 'Object literal ...')];
+    expect(selectFatalDiagnostics(withError)).toHaveLength(1);
+    expect(() => hooks.afterDiagnostic(withError)).toThrowError(/TS2353/);
+  });
+
+  it('names file, line, column and code, relative to the package', () => {
+    const { diagnostics, dir } = diagnosticsFor("export const answer: number = 'nope';\n");
+    const [diagnostic] = selectFatalDiagnostics(diagnostics);
+
+    expect(formatDiagnostic(diagnostic, dir)).toMatch(/^probe\.ts\(1,14\): error TS2322: /);
+    // Absolute when the diagnostic is outside the package being built, rather
+    // than a path computed against the wrong root.
+    expect(formatDiagnostic(diagnostic, '/somewhere/else')).toMatch(/^\/.*probe\.ts\(1,14\): /);
+  });
+
+  it('quotes at most `maxQuoted` diagnostics and says how many it elided', () => {
+    const many = Array.from({ length: 7 }, (_, index) =>
+      bare(ts.DiagnosticCategory.Error, 2300 + index, `error number ${index}`)
+    );
+
+    const message = describeFatalDiagnostics(many, { packageDir: '/pkg', maxQuoted: 3 })!;
+    expect(message).toContain('reported 7 type errors');
+    expect(message).toContain('TS2300');
+    expect(message).toContain('TS2302');
+    expect(message).not.toContain('TS2303');
+    expect(message).toContain('... and 4 more');
+  });
+
+  it('returns null rather than an empty message when nothing is fatal', () => {
+    expect(describeFatalDiagnostics([], { packageDir: '/pkg' })).toBeNull();
+    expect(
+      describeFatalDiagnostics([bare(ts.DiagnosticCategory.Warning, 6133, 'unused')], {
+        packageDir: '/pkg',
+      })
+    ).toBeNull();
+  });
+
+  it('shares no hook name with createDtsExplicitExtensions', () => {
+    // `packages/layout/vite.config.ts` spreads both factories into ONE object
+    // literal. Overlapping keys there would silently drop whichever came
+    // first — no error, no log line, and one of the two fixes just stops
+    // existing. If a hook is ever added to either factory, this fails and
+    // points at the call site that has to compose instead of spread.
+    const failOnErrors = Object.keys(createDtsFailOnTypeErrors({ packageDir: '/pkg' }));
+    const explicitExtensions = Object.keys(createDtsExplicitExtensions({ packageDir: '/pkg' }));
+
+    expect(failOnErrors).toEqual(['afterDiagnostic']);
+    expect(explicitExtensions.sort()).toEqual(['afterBuild', 'beforeWriteFile']);
+    expect(failOnErrors.filter((hook) => explicitExtensions.includes(hook))).toEqual([]);
+  });
+});

scripts/vite-dts-fail-on-type-errors.ts

@@ -0,0 +1,174 @@
+// An honest exit code for the declaration half of a `vite build`.
+//
+// ## The defect (objectui#5370)
+//
+// `vite-plugin-dts` builds a full TypeScript program to emit `dist/**/*.d.ts`,
+// computes that program's diagnostics, PRINTS them — and then lets the build
+// finish green:
+//
+//     src/AppSchemaRenderer.tsx:581:5 - error TS2353: Object literal may only
+//       specify known properties, and 'logo' does not exist in type
+//       'AppShellBranding'.
+//     [unplugin:dts] Declaration files built in 4349ms.
+//     ✓ built in 4.62s
+//     BUILD_EXIT=0            <- ${PIPESTATUS[0]}, i.e. pnpm's own exit code
+//
+// Measured on `@object-ui/layout` at `origin/main` `478ec54ce` with one
+// deliberate in-source type error. The diagnostic is right there in the log,
+// and the exit code — the thing scripts, `turbo`, CI steps and agents actually
+// branch on — says success.
+//
+// ## Why this is worth a hook rather than a shrug
+//
+// CI is not blind: `type-check` is a separate task and it goes red, so broken
+// source does not reach `main` through this. The exposure is to whoever judges
+// a package from `build` alone, which is a normal thing to do mid-task: change
+// a shared type, rebuild the package so consumers resolve the fresh `.d.ts`,
+// see `✓ built` and `EXIT=0`, conclude the package is clean. It is not — and a
+// `| tail` or a `grep` for the summary line scrolls the one honest line out of
+// view. A build that has already computed the diagnostic and chosen to print it
+// is one flag away from also refusing to exit 0.
+//
+// That is the whole of this module: it does not compute anything new, it does
+// not duplicate `type-check`, and it must never become a second, weaker copy of
+// it. It converts a diagnostic the build ALREADY HAS into the exit code the
+// build already implies.
+//
+// ## Three properties held on purpose
+//
+// - **Errors only.** `category === ts.DiagnosticCategory.Error`. Warnings,
+//   suggestions and messages stay non-fatal; a gate that fails on a suggestion
+//   is a gate someone disables, and disabling it takes the errors with it.
+// - **The message carries the diagnostics.** The plugin logged them once, at
+//   program creation, hundreds of lines above the failure. Repeating them in
+//   the thrown error is not noise — the scrolled-away log line is precisely the
+//   defect this module is about, so the failure has to be readable on its own.
+// - **One hook, no overlap.** This factory returns `afterDiagnostic` and
+//   nothing else, so it can be spread beside
+//   `scripts/vite-dts-explicit-extensions.ts` (`beforeWriteFile` + `afterBuild`)
+//   without either silently overwriting the other. A duplicate key in an object
+//   literal keeps the last one and says nothing.
+//   `scripts/__tests__/vite-dts-fail-on-type-errors.test.ts` pins that
+//   disjointness so a later hook added here cannot quietly break a call site.
+//
+// ## What this does NOT reach
+//
+// `afterDiagnostic` runs at `writeBundle`, before the declaration emit, and is
+// handed the diagnostics the plugin computed when it created the program
+// (declaration + semantic + syntactic). Diagnostics raised BY the emit itself —
+// the `emitSkipped` path inside the plugin's runtime — are appended after this
+// hook has run and are therefore outside its reach; the plugin still prints
+// them, and the emitted-output assertions in
+// `scripts/vite-dts-explicit-extensions.ts` catch the shape where that path
+// produces no declarations at all. Widening to that path needs a post-emit
+// hook, which collides with `afterBuild` and is deliberately not done here.
+//
+// Because the throw happens before the emit, a package with type errors also
+// stops writing NEW typings — `noEmitOnError` semantics for the declaration
+// leg. On a healthy tree (measured: all 22 `vite-plugin-dts` packages on
+// `478ec54ce` emit zero diagnostics) nothing changes at all.
+
+import ts from 'typescript';
+
+/** How many diagnostics the failure message quotes before it truncates. */
+const DEFAULT_MAX_QUOTED = 10;
+
+export interface DtsFailOnTypeErrorsOptions {
+  /**
+   * Absolute path of the package root, used to name the package in the failure
+   * message and to print diagnostic paths relative to it.
+   */
+  packageDir: string;
+  /** How many diagnostics to quote in the failure message. Defaults to 10. */
+  maxQuoted?: number;
+}
+
+/** The single `vite-plugin-dts` hook that carries the fix. */
+export interface DtsFailOnTypeErrorsHooks {
+  afterDiagnostic: (diagnostics: readonly ts.Diagnostic[]) => void;
+}
+
+/**
+ * The diagnostics that make a build wrong, out of everything the type program
+ * reported.
+ *
+ * Split out from the hook so the predicate can be pinned directly: the
+ * difference between "fails on errors" and "fails on anything TypeScript felt
+ * like mentioning" is not visible in a passing build.
+ */
+export function selectFatalDiagnostics(
+  diagnostics: readonly ts.Diagnostic[]
+): ts.Diagnostic[] {
+  return diagnostics.filter((diagnostic) => diagnostic.category === ts.DiagnosticCategory.Error);
+}
+
+/** One diagnostic as `relative/file.ts(line,col): error TS1234: message`. */
+export function formatDiagnostic(diagnostic: ts.Diagnostic, packageDir: string): string {
+  const message = ts.flattenDiagnosticMessageText(diagnostic.messageText, ' ');
+  const code = `error TS${diagnostic.code}`;
+
+  if (!diagnostic.file || diagnostic.start === undefined) {
+    return `${code}: ${message}`;
+  }
+
+  const { line, character } = diagnostic.file.getLineAndCharacterOfPosition(diagnostic.start);
+  // `path.relative` is deliberately not used: the diagnostic's file name is
+  // already normalized to forward slashes by the plugin, and a relative path is
+  // only cosmetic here — a prefix strip cannot fail the way a path computation
+  // against the wrong root can.
+  const file = diagnostic.file.fileName.startsWith(`${packageDir}/`)
+    ? diagnostic.file.fileName.slice(packageDir.length + 1)
+    : diagnostic.file.fileName;
+
+  return `${file}(${line + 1},${character + 1}): ${code}: ${message}`;
+}
+
+/**
+ * The failure message for a set of fatal diagnostics, or `null` when there are
+ * none.
+ *
+ * Exported so the wording — the half of this module a human actually reads when
+ * it fires — is pinned by a test rather than by nobody.
+ */
+export function describeFatalDiagnostics(
+  diagnostics: readonly ts.Diagnostic[],
+  options: DtsFailOnTypeErrorsOptions
+): string | null {
+  const fatal = selectFatalDiagnostics(diagnostics);
+  if (fatal.length === 0) return null;
+
+  const packageDir = options.packageDir.replace(/\\/g, '/').replace(/\/$/, '');
+  const maxQuoted = options.maxQuoted ?? DEFAULT_MAX_QUOTED;
+  const quoted = fatal.slice(0, maxQuoted).map((d) => `  ${formatDiagnostic(d, packageDir)}`);
+  const elided = fatal.length - quoted.length;
+
+  return (
+    `[dts-fail-on-type-errors] the declaration build of \`${packageDir}\` reported ` +
+    `${fatal.length} type error${fatal.length === 1 ? '' : 's'}, so it must not exit 0:\n` +
+    quoted.join('\n') +
+    (elided > 0 ? `\n  ... and ${elided} more` : '') +
+    `\n\nThe declaration emit was not attempted. Fix the errors, or run ` +
+    `\`pnpm --filter <package> type-check\` for the full report.`
+  );
+}
+
+/**
+ * Wire fail-on-type-errors into a `vite-plugin-dts` invocation.
+ *
+ * ```ts
+ * dts({
+ *   ...existing,
+ *   ...createDtsFailOnTypeErrors({ packageDir: __dirname }),
+ * })
+ * ```
+ */
+export function createDtsFailOnTypeErrors(
+  options: DtsFailOnTypeErrorsOptions
+): DtsFailOnTypeErrorsHooks {
+  return {
+    afterDiagnostic(diagnostics) {
+      const failure = describeFatalDiagnostics(diagnostics, options);
+      if (failure !== null) throw new Error(failure);
+    },
+  };
+}