fix(build): make dts type errors fail the build instead of exiting 0 - #5482
Merged
Conversation
`vite-plugin-dts` builds a full type program, computes its diagnostics, prints them, and lets the build finish green. Measured on `@object-ui/layout` at `478ec54ce` with one deliberate TS2353: the error is in the log, `vite build` exits 0, and anything branching on that exit code reads the package as clean. `scripts/vite-dts-fail-on-type-errors.ts` turns the diagnostics the build already has into the exit code it already implies, via the plugin's documented `afterDiagnostic` hook. Errors only; warnings stay non-fatal. The thrown message quotes the diagnostics because the plugin logged them hundreds of lines earlier, which is the defect this is about. Wired into `packages/layout` beside the existing explicit-extensions factory; the two return disjoint hooks so neither spread can overwrite the other. Part of #5370
Feeds the subject diagnostics from a real `ts.createProgram` rather than hand-rolled objects, so the predicate is pinned against the shape the plugin actually hands it. Also pins that the two dts factories share no hook name — `packages/layout/vite.config.ts` spreads both into one object literal, where a duplicate key silently keeps the last one. Part of #5370
Build tooling only; no package src/ changes and nothing to publish. Part of #5370
Contributor
✅ Console Performance Budget
The eager closure is every chunk the entry reaches through static imports — what the browser fetches and parses before the app renders. The entry chunk on its own is a small fraction of it. 📦 Bundle Size Report
Size Limits
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Part of #5370.
The mechanism lands here and
@object-ui/layout— the package the card measured — is wired toit. The other 20 packages that still exit 0 on a dts type error each need a one-line spread in
their own
vite.config.ts, which is outside this PR's declared file surface (the sharedvite/dts config +
packages/layout). Deliberately notFixes, so merging this does not closea card that is only one package deep. The width reading and the follow-up are below.
Before / after, measured
Both readings are
${PIPESTATUS[0]}— pnpm's own exit code, never a pipe's.Before —
origin/main478ec54ce, plus one deliberate TS2353 inpackages/layout/src/AppSchemaRenderer.tsx(the card's own injection: alogokey on an objectliteral typed
AppShellBranding):Control on that same tree — the error IS fatal where a gate looks at it:
pnpm --filter @object-ui/layout type-check→ exit 2.After — this branch, same injected error, same command:
Unmodified source with the fix wired:
BUILD_EXIT=0,✓ built in 5.03s— unchanged.The failure message repeats the diagnostics on purpose. The plugin logs them once, at program
creation, hundreds of lines above the failure, and a
| tailscrolling them out of view ishalf of what the card is about.
Blast radius — nothing currently red, measured rather than assumed
22 packages build their typings through
vite-plugin-dts. On unmodified478ec54ce, everyone of them was built with
pnpm --filter PACKAGE buildand its log scanned (ANSI stripped) forTypeScript diagnostics:
22 / 22 exit 0 with zero diagnostics of any category. So making dts errors fatal reds
nothing that is green today — for the one package wired here, and for the 20 that a follow-up
would wire.
Every one of those 22 zeros is counter-probed rather than trusted: a second sweep appended one
deliberate
TS2322to each package's entry file and rebuilt. All 22 reported it, so no zeroabove is the vacuous kind where a package's dts program never looked at its own
src.That second sweep is also the width reading, in one table (exit code with the probe in place):
@object-ui/layout@object-ui/fieldstsc && vite build && …, so the leadingtscfails first and the dts leg never runscomponents,plugin-ai,plugin-calendar,plugin-charts,plugin-chatbot,plugin-dashboard,plugin-designer,plugin-detail,plugin-editor,plugin-form,plugin-gantt,plugin-grid,plugin-kanban,plugin-list,plugin-map,plugin-markdown,plugin-report,plugin-timeline,plugin-tree,plugin-view)Reach — what "the shared vite/dts config" actually is today
Measured, not assumed from the filter list: there is no shared vite config in this repo. Each
of the 22 packages calls
dts({ … })in its ownvite.config.ts. The only shared dts module isscripts/vite-dts-explicit-extensions.ts, imported by exactly 2 of them (components,layout).turbo.jsonalready treats$TURBO_ROOT$/scripts/vite-*.tsas a build input, so anew module there invalidates the right caches on its own.
So "fix it once in the shared config" can only mean: implement once under
scripts/, then spreadit at each call site. This PR does the first half and one call site. Wiring the remaining 20 is
mechanical and measured safe, but it is 20 files this seat was not given, and a gate that keeps
them wired (so a new package cannot land unwired) is a further step — filed separately.
Why
afterDiagnostic, and what it does not reachvite-plugin-dtsdocumentsafterDiagnosticfor exactly this ("according to thediagnostics.length, you can judge whether there is any type error"). It is also the only hookthe two shared factories do not both want: this one returns
afterDiagnosticand nothing else,so it spreads beside
createDtsExplicitExtensions'sbeforeWriteFile/afterBuildwith no keycollision — a duplicate key in an object literal keeps the last one and reports nothing. A test
pins that disjointness so a hook added to either factory later goes red at the pin instead of
silently at the call site.
Errors only (
ts.DiagnosticCategory.Error); warnings and suggestions stay non-fatal. Diagnosticsraised by the emit itself (the plugin's
emitSkippedpath, appended after this hook has run) areoutside its reach and the module header says so.
Reverse-verification
Predictions recorded before running; both matched.
card's exact defect returns —
BUILD_EXIT=0, plugin still prints TS2353, no[dts-fail-on-type-errors]line. Observed: exactly that (0 / printed / 0 hook lines).Restored:
BUILD_EXIT=1again.4 failed / 3 passed (cases 1, 3, 4, 5 depend on it; 2, 6, 7 do not). Observed:
Tests 4 failed | 3 passed (7). Restored: 7 passed.No rebuild leg is involved in either: both the vite config and the test import the subject
through a relative source specifier (
../../scripts/vite-dts-fail-on-type-errors.ts), notthrough a package's
exportsinto adist/, so there is no stale-artifact path that could keepan ablation alive. Ablation 1 changing behaviour is itself the proof of that.
Tree clean after both (
git status --porcelainempty).Gates run locally, on
a15034dffpnpm type-check:scripts·node scripts/check-type-check-coverage.mjs·node scripts/check-lint-coverage.mjs·pnpm check:control-bytes·pnpm exec eslinton both new files ·pnpm --filter @object-ui/layout lint·pnpm --filter @object-ui/layout type-check·pnpm --filter @object-ui/layout build·node scripts/check-changeset-presence.mjs·check-changeset-no-major.mjs·check-changeset-fixed.mjs·pnpm exec vitest run scripts/__tests__/…(25 passed).All exit 0.
Changeset is an empty frontmatter: build tooling only, no package
src/touched, nothing topublish — declared explicitly rather than left undeclared.
No test was skipped, disabled or quarantined, and the
type-checkleg is untouched.Generated by Claude Code