Uh oh!
There was an error while loading. Please reload this page.
fix(scripts): parse tsconfigs with TypeScript's reader instead of three regexes - #5938
Merged
Merged
Conversation
…exes `readTsconfig` in the ESM-load gate blanked comments with three ordered `replace` calls before `JSON.parse`. None of them knew what a JSON string was, so a slash-star inside a `paths` key opened a "block comment" that ran to the next star-slash — usually the test glob in `exclude` at the bottom of the same file — and deleted the configuration between them. Measured on ed35c23 over the 91 tsconfigs tracked by git: 61 threw. Every throw was absorbed by `effectiveNoEmit`'s catch, which grades an unreadable config as emitting, so this gate's scope was decided by a fallback rather than by a reading, with no signal. Re-measured with the fix: 0 of 91 throw. `ts.parseConfigFileTextToJson` is TypeScript's own tsconfig reader. No new dependency edge — `typescript` is a root devDependency and this module already loads it transitively through the scanner it imports from `check-phantom-dependencies.mjs`. Scope consequence, deliberate and measured: `@object-ui/fields` leaves the specifier leg (13 specifier-preserving packages become 12). Its `tsc` step inherits the root's `noEmit: true` and only type-checks; `dist` comes from vite. It carries 0 extensionless specifiers in sources and 0 of 155 relative specifiers in `dist`, and its tsconfig pins `nodenext`, so the compiler rejects a missing extension (TS2835) in its own build. `@object-ui/auth`'s verdict is unchanged and merely stops being an accident. Ships the class-closing assertion: every tsconfig in the repository round-trips through `readTsconfig` without throwing, with a floor on the walk so an empty scan cannot pass as a clean one. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PQ3NihCHE9LUtHoGxo6A9f
os-warren
marked this pull request as ready for review
August 24, 2026 02:43
Uh oh!
There was an error while loading. Please reload this page.
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 freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Fixes#5367
readTsconfig()in the ESM-load gate blanked comments with three orderedreplacecalls and handed the result toJSON.parse. None of them knew what a JSON string was, so a slash-star written inside a string opened a "block comment" that ran to the next star-slash anywhere in the file and deleted every line between them.This is the same class as the specifier mask #5382 retired in this same file, and #5382's own landed code named this as its remainder (
scripts/check-node-esm-load.mjs:91,:404). Both of those comments are updated here.The defect is much larger than the card measured
The card was filed for two configs. Measured on
mainated35c23bb, over the 91 tsconfigs tracked by git:maints.parseConfigFileTextToJson"@/*"in apathsmap and"**/*.test.ts"in anexcludelist are the two commonest lines in this repository's tsconfigs — and they are a matched comment opener and closer. Every one of those 61 throws was absorbed byeffectiveNoEmit's catch, which grades an unreadable config as emitting. So this gate's scope was decided by a fallback rather than by a reading, with no signal that it had happened.The fix
ts.parseConfigFileTextToJsonis TypeScript's own tsconfig reader: comments, trailing commas and BOMs stop being questions this gate has an opinion about, and the answer is by construction the onetscitself computes.No new dependency edge, verified rather than inherited — the previous seat recorded that it had not checked this:
typescriptis a root devDependency (^6.0.3).check-phantom-dependencies.mjs, whose first import isimport ts from 'typescript'.check:phantom-depsexits 0 and printsEvery in-scope import is declared by the package that publishes it.Its own output shows why it has no opinion here: it scans40 released package(s), and rootscripts/is not one of them.readTsconfigstill throws on a genuinely unparseable config. That is deliberate — it is the contracteffectiveNoEmit's conservative catch is written against, and the contract the new test asserts across the repository.The embedded scope question, answered on the record
Parsed correctly, the gate's scope moves exactly once, and this is the named consequence:
Visible in the gate's own first line, measured both ways in one sitting:
It is allowed to drop, and here is the measurement behind that.
packages/fields/tsconfig.jsoninherits the root'snoEmit: true, and its own comment says so in as many words: "this config inherits the root'snoEmit: true, sotschere only CHECKS;distis written by vite-plugin-dts." The gate graded it "emitting" only because the parse threw.fields' sources carry 0 extensionless relative specifiers today.packages/fieldsand scanned its artifact with the gate's ownrelativeSpecifiers: 0 extensionless out of 155 relative specifiers indist. Rolldown resolves them while bundling, so the property this leg ratchets is not observable in whatfieldspublishes."moduleResolution": "nodenext", under which a missing relative extension is TS2835. Verified by ablation rather than asserted — see below.@object-ui/authis the other half of the move and does not change: its config throws under the old reader too, but it declares"noEmit": falseoutright, so its verdict is unchanged and merely stops being an accident. Both halves are now pinned by tests, so a future change to this membership has to come with a decision.Also corrected:
buildPreservesSpecifiers' doc named@object-ui/fieldsas its emitting-tsc-plus-bundler example. That example was a consequence of the broken parse. The rule is kept (the shape is legal) with the example removed and the correction recorded — verified that no published package matches that shape today.The class-closing assertion
Walked rather than listed (a hand-maintained list answers "the files someone remembered", which is how two poisoned configs sat unnoticed), using the sibling gates' own
SKIP_DIRS. The walk finds exactly the same 91 files asgit ls-files '*tsconfig*.json', and it asserts its own size (floor of 60) so an empty scan cannot pass as a clean one.Five more pins ship with it: the
paths-key shape, the line-comment-prose shape, the still-throws contract, and the two scope verdicts above.Reverse verification
Ablation A — restore the three-regex strip, keep the new tests. Mutation confirmed on disk against the exact text targeted (
JSON.parse(raw)count 1 injected;parseConfigFileTextToJsoncount 0 in code, only the doc mention left). Nothing is rebuilt in this path: node and vitest loadscripts/*.mjsfrom source and this gate has nodistcopy. Restore ran from anEXIT INT TERMtrap and the tree was re-verified clean afterwards.The five reds are the round-trip test and the four shape/scope pins. The sixth new test — "still THROWS on a config nobody can read" — stays green under the ablation, and that is the honest direction: the old reader also throws on genuinely broken JSON, so that pin guards the contract, not the defect. The gate itself printed
13 of 39under the ablation and prints12 of 39with the fix.Ablation B — the claim that
fieldsloses no coverage. Replaced one'./widgets/TextField.js'with'./widgets/TextField'inpackages/fields/src/FieldEditWidget.tsx(on-disk proof: removed-text count 0, injected-text count 1).tscreadssrcdirectly, so again nothing is rebuilt.That is precisely the coverage the drop costs and precisely the coverage the compiler replaces — inside
fields' ownbuildscript (tsc && vite build && …), not in a separate job. Restored by trap.Gates, each by name with its own verdict
Union re-run at
f04e306, working tree clean (git status --porcelain→ 0 lines). Exit codes captured by direct redirection, never after a pipe.pnpm exec vitest run scripts/Test Files 61 passed (61)/Tests 1637 passed (1637)check:esm-specifiersSpecifier leg: no un-ledgered package emits an extensionless relative specifier.check:phantom-deps✅ Every in-scope import is declared by the package that publishes it.check:control-bytes✅ check-control-bytes: OK (scanned 4928 tracked text file(s); skipped 85 binary).check:type-check-coverage✅ test type-check coverage: 41/41 packages compile their tests, 0 declared debt…check:self-import✅ No package names itself inside its own src/.type-check:scripts(tsc -p tsconfig.scripts.json)check-changeset-presence.mjs✅ No source of a released package changed in this range, so no changeset is owed.check-changeset-no-major.mjs✅ No changeset declares amajorbump.No changeset, following
check-changeset-presence.mjs's own verdict: the diff is two files under rootscripts/, and0 of them under the src/ of a package the release covers.Declared narrowings
files linted: 2, read from--format json, with no "file ignored" warning), and type-aware linting is not enabled (eslint.config.jscontains noprojectService, noparserOptions, noproject:), so this diff cannot move the verdict on any untouched file. Result: 0 errors, 0 warnings. The repo-wide run is CI's.check:node-esm-loadbuilds every published package; its own CI job (.github/workflows/node-esm-load-gate.yml→Build every published package and import its entry under plain Node) owns it. The narrowing is safe by construction: leg 2 iteratesesmPackages(every published ESM package) and never consultsbuildPreservesSpecifiers, so nothing in this diff can move it. Leg 1 — the only leg this diff can move — was run standalone and is green.Scope
scripts/check-node-esm-load.mjs+scripts/__tests__/check-node-esm-load.test.ts, the declared file surface. No breach.Checked while here, filed nothing because there was nothing to file:
scripts/check-type-check-coverage.mjshas its ownreadTsconfig, but itsstripJsonCommentsis a real character scanner that tracks strings and escapes — measured over all 91 configs, 0 unparseable. No other script in the repository reads a tsconfig from disk. This was the last regex-based tsconfig reader.Generated by Claude Code