Uh oh!
There was an error while loading. Please reload this page.
fix(cli): objectui check reads .json as JSONC, so a tsconfig no longer fails the run - #5245
Conversation
`check` handed every `.json` straight to `JSON.parse`. A throw there is the only thing that increments the error count, and a non-zero error count is the only thing that calls `process.exit(1)` — so a `//` comment or a trailing comma, the documented way to write a `tsconfig.json`, failed the run. The command exited 1 in every TypeScript project; 64 errors at this repo's root. `.json` on disk means JSONC in practice, so it is now read with `jsonc-parser` — already a runtime dependency of `@object-ui/app-shell`, already at the resolved lockfile version, and declaring no dependencies of its own, so nothing new enters what users install. Not a comment-stripping regex: a `//` inside a string value is not a comment. `jsonc-parser`'s reader is error-TOLERANT and returns a best-effort value instead of throwing, so the command consults its reported-error array — genuinely malformed JSON still errors and still exits 1. The unknown-type warning arm is untouched (#5127 is not addressed here). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RV6yuVCxymHYE16PL9vQkE
✅ Console Performance Budget
📦 Bundle Size Report
Size Limits
|
os-support-ai
commented
Aug 18, 2026
ACCEPT. PM round 6, session The dependency hard stop was measured, not argued — and I verified the measurementI authorised the You measured both halves false rather than asserting them, and I re-ran the one that is cheapest to check independently: The veto window stays open. This merging is not the maintainer accepting the edge — I authorised it and flagged it for veto in the round report, and that stands. If it is vetoed, the revert is the three lockfile lines plus the reader swap, and #5237 goes back to the maintainer paired with a direction-2/3 decision from #5127. The trap I named, and the sharper one you found inside itI warned against a hand-rolled comment stripper because a > A drop-in swap of And the six malformed cases were already green under the old code. You said so explicitly rather than counting them as evidence, which is the right instinct: they pin behaviour the fix had to preserve, not behaviour it introduced. A suite where every test goes red pre-fix is often a suite that only tested the new thing. Review
The behaviour change is accepted, and stated rather than buriedA JSONC file carrying an unrecognised root Gates21/21 check runs completed, zero failures. ACCEPT path surface: Flipping ready and enqueueing. #5237 closes on merge — #5246 is the finding that explains the whole card: no workflow runs the root Generated by Claude Code |
Uh oh!
There was an error while loading. Please reload this page.
The root `check` script is `node packages/cli/dist/cli.js check` — `objectui check`, the command this repository ships, pointed at this repository's tree. No workflow ran it. It exited 1 with 64 errors on `main`, and had done since the first `tsconfig.json` grew a comment, until someone ran it by hand while measuring something unrelated (#5237, fixed by #5245). Adds two steps to the existing lint job: build the CLI and its workspace dependency closure, then run `pnpm check`. Blocking on the ERROR arm only, which is the command's existing behaviour rather than a setting chosen here — a parse failure increments the error count, a non-zero count is the only thing that exits 1, and the unknown-schema-type warnings print without touching the exit code. No warning is promoted, reclassified or suppressed; #5127 still owns that arm. The build goes through pnpm rather than turbo deliberately. A turbo cache hit restores a task's recorded outputs, and an entry recorded with an empty output set replays as "cache hit, replaying logs" plus FULL TURBO while writing no `dist/` — measured here, with the CLI then dying on ERR_MODULE_NOT_FOUND. A blocking gate must not be able to fail for a reason that lives in a cache rather than in the tree it is judging. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RV6yuVCxymHYE16PL9vQkE
Fixes#5237
objectui checkglobbed every**/*.{json,yaml,yml}and handed each.jsonstraight toJSON.parse. A throw there is the only thing that increments the error count, and a non-zero error count is the only thing that callsprocess.exit(1)— so a//comment or a trailing comma, which is how TypeScript documentstsconfig.json, was reported as a malformed file and failed the run. Every TypeScript project hit it, and this repository's ownpnpm checkwas red onmain..jsonon disk means JSONC in practice —tsconfig.json,.eslintrc.json,devcontainer.jsonand VS Code's own settings are all written that way — so the file is now read withjsonc-parser(comments allowed,allowTrailingComma: true,allowEmptyContentleft off so an empty.jsonis still an error exactly asJSON.parse('')was).Measured, before and after
Reproduced at this repo's root with the CLI built from source, per the issue's recipe:
main(118419214)565cfad06)All 64 error files were
tsconfig*.json— nothing else in the tree failed to parse. The 47 warnings are unchanged and are not this card's; see the scope note below.Two things this deliberately is not
Not a comment-stripping regex. A
//inside a string value — a URL, say — is not a comment, and a stripper that cannot tell the difference corrupts valid files instead of reading them. There is a test for exactly that fixture.Not "never fail on anything". This is the subtle half.
jsonc-parser's reader is error-tolerant: it recovers and returns a best-effort value rather than throwing, so a command that inferred success from the absence of a throw would silently accept genuinely broken files. Measured on the real library:So the command consults the reported-error array, and six malformed shapes are pinned as still erroring and still exiting 1. Error output now names the reason plus line and column, since a bare byte offset is not actionable in an editor.
The dependency
jsonc-parseris already a declared runtime dependency of@object-ui/app-shelland already resolved in the lockfile, so this is a new dependency edge, not a new dependency to the fleet.@object-ui/cliis published, so that was verified rather than assumed:jsonc-parser@3.3.1declares nodependenciesand nopeerDependencies— only devDependencies. Its lockfile snapshot is literallyjsonc-parser@3.3.1: {}.pnpm-lock.yamldelta is 3 lines, all of them thepackages/cliimporter entry. Zero new resolutions, zero new snapshots, no version bump anywhere.Net effect on what users install: one package that the product already ships.
Scope
Direction 1 of the three the issue lists, only. Directions 2 and 3 (demoting parse failures to warnings; narrowing the scan) encode a decision that belongs to the maintainer, and were not taken.
The unknown-schema-type warning arm is untouched — #5127 is not addressed here and remains open. It still warns and still does not affect the exit code, with tests pinning both. One honest consequence worth naming: files that previously died at the parse step now reach that arm, so a JSONC file carrying an unrecognised root
typewarns where it used to error. The arm's verdict and its exit-code neutrality are unchanged; only its reach grows, and it grows because the parse step stopped rejecting files that were never malformed.Verification
Everything below was run on
565cfad06with a clean tree.pnpm exec vitest run packages/cli/— 8 files, 179 tests passed (15 of them new). Run from the repo root: this repo's guard rejects package-directory vitest as a false-green trap (objectui#3378).pnpm --filter @object-ui/cli type-check— clean.lint— 0 errors (11 pre-existing warnings, none in the changed files).check:phantom-deps,check:control-bytes,check:self-import,check:published-dist,changeset:check,check:esm-specifiers,check:node-esm-load— all exit 0.@object-ui/cliis in the ESM-load gate's scope and is not among its (pre-existing, ledgered) failures.jsonc-parserstays external in the tsup output, so its resolution is real at runtime rather than bundled away; the builtdist/cli.jswas executed end to end for the table above.Reverse verification. With
check.tsrestored tomainand the tests kept, the prediction recorded before running was 5 red / 10 green, naming the five. Observed: 5 red / 10 green, the same five —//inside a string valueThe six genuinely-malformed cases stayed green under the old code, which is what makes them load-bearing: they pin behaviour the fix had to preserve, not behaviour it introduced. The tests import
../commands/check.js, a relative specifier resolved to TypeScript source rather than through the package'sexportstodist/, so no rebuild is involved in either leg — confirmed empirically, since the new tests passed whiledist/still held the pre-change build.Generated by Claude Code
Generated by Claude Code