fix(scripts): parse tsconfigs with TypeScript's reader instead of three regexes

scripts/__tests__/check-node-esm-load.test.ts

@@ -2,6 +2,7 @@ import { describe, expect, it } from 'vitest';
 import fs from 'node:fs';
 import os from 'node:os';
 import path from 'node:path';
+import { fileURLToPath } from 'node:url';
 
 import {
  ESM_MODULE_EDGE,
@@ -16,10 +17,12 @@ import {
  emittedSources,
  esmEntryOf,
  importEntry,
+ readTsconfig,
  relativeSpecifiers,
  resolvesToModule,
  scanSpecifiers,
 } from '../check-node-esm-load.mjs';
+import { SKIP_DIRS } from '../check-phantom-dependencies.mjs';
 
 /**
  * objectui#4538 — a published entry plain Node ESM cannot load.
@@ -199,6 +202,182 @@ describe('scope — which builds preserve specifiers', () => {
  });
 });
 
+/**
+ * objectui#5367 — the tsconfig reader was the same class as the specifier mask.
+ *
+ * `readTsconfig` stripped comments with three ordered regexes and handed the
+ * result to `JSON.parse`. None of them knew what a JSON string was, so the
+ * slash-star inside a `paths` key opened a "block comment" that ran to the next
+ * star-slash — typically the test glob in `exclude` at the bottom of the same
+ * file — and deleted everything between. Measured before the fix: 61 of this
+ * repository's 91 tsconfigs threw, and every throw was swallowed by
+ * `effectiveNoEmit`'s catch, which grades an unreadable config as emitting.
+ *
+ * The gate's verdict happened to be safe. It was also never computed, which is
+ * the property this block exists to keep from coming back.
+ */
+
+const REPO_ROOT = path.resolve(path.dirname(fileURLToPath(import.meta.url)), '../..');
+const TSCONFIG_NAME = /^tsconfig(\..+)?\.json$/;
+
+/**
+ * The slice of a parsed tsconfig these assertions read.
+ *
+ * `readTsconfig` returns arbitrary JSON, so the narrowing is written down here
+ * once rather than asserted inline at every call — and it stays `unknown` at the
+ * leaves, so a wrong assumption about a value's shape is still a type error.
+ */
+type ParsedTsconfig = {
+ compilerOptions?: Record<string, unknown>;
+ extends?: unknown;
+ include?: unknown;
+ exclude?: unknown;
+};
+
+const parseTsconfig = (file: string) => readTsconfig(file) as ParsedTsconfig;
+
+/**
+ * Every tsconfig in the repository, walked rather than listed.
+ *
+ * A hand-maintained list would answer "the files someone remembered", which is
+ * exactly the reading that let two poisoned configs sit unnoticed. `SKIP_DIRS`
+ * is the sibling gates' own exclusion set, so `node_modules` and build output
+ * are out for the same reason they are out everywhere else.
+ */
+function everyTsconfig(dir: string, out: string[] = []): string[] {
+ for (const entry of fs.readdirSync(dir, { withFileTypes: true })) {
+ if (entry.isDirectory()) {
+ if (SKIP_DIRS.has(entry.name) || entry.name.startsWith('.')) continue;
+ everyTsconfig(path.join(dir, entry.name), out);
+ } else if (TSCONFIG_NAME.test(entry.name)) {
+ out.push(path.join(dir, entry.name));
+ }
+ }
+ return out;
+}
+
+describe('readTsconfig reads tsconfigs rather than pattern-matching around them', () => {
+ it('round-trips EVERY tsconfig in the repository without throwing', () => {
+ const configs = everyTsconfig(REPO_ROOT);
+
+ // The size assertion is the same doctrine the gate applies to itself: a walk
+ // that found nothing would throw nothing and report success. 91 exist today;
+ // 60 is a floor that a real collapse trips and ordinary churn does not.
+ expect(configs.length).toBeGreaterThanOrEqual(60);
+
+ const threw: string[] = [];
+ for (const file of configs) {
+ try {
+ readTsconfig(file);
+ } catch (error) {
+ threw.push(`${path.relative(REPO_ROOT, file)}: ${(error as Error).message}`);
+ }
+ }
+ expect(threw).toEqual([]);
+ });
+
+ it('keeps a `paths` key whose value opens a comment, and the keys after it', () => {
+ // The exact two lines that poisoned `packages/auth/tsconfig.json`: the
+ // slash-star in the paths key, and the test glob in `exclude` that closed
+ // the comment it opened. The old strip returned a config with no `paths`,
+ // no `include`, and a truncated `exclude`.
+ const dir = tmpdir();
+ fs.writeFileSync(
+ path.join(dir, 'tsconfig.json'),
+ [
+ '{',
+ ' "compilerOptions": {',
+ ' "baseUrl": ".",',
+ ' "paths": { "@/*": ["src/*"] },',
+ ' "noEmit": false',
+ ' },',
+ ' "include": ["src"],',
+ ' "exclude": ["node_modules", "dist", "**/*.test.ts", "**/*.test.tsx"]',
+ '}',
+ '',
+ ].join('\n'),
+ );
+
+ const config = parseTsconfig(path.join(dir, 'tsconfig.json'));
+ expect(config.compilerOptions?.paths).toEqual({ '@/*': ['src/*'] });
+ expect(config.include).toEqual(['src']);
+ expect(config.exclude).toEqual(['node_modules', 'dist', '**/*.test.ts', '**/*.test.tsx']);
+ expect(config.compilerOptions?.noEmit).toBe(false);
+ });
+
+ it('keeps the config below a line comment whose prose contains a slash-star', () => {
+ // `packages/fields/tsconfig.json`'s shape: a `//` comment naming a package
+ // glob. The block-comment pass ran first and had no notion of being inside a
+ // line comment, so the glob opened a comment that ate the rest of the file —
+ // which is how a config that plainly inherits `noEmit: true` was graded as
+ // emitting for as long as this gate has existed.
+ const dir = tmpdir();
+ fs.writeFileSync(path.join(dir, 'base.json'), JSON.stringify({ compilerOptions: { noEmit: true } }));
+ fs.writeFileSync(
+ path.join(dir, 'tsconfig.json'),
+ [
+ '{',
+ ' "extends": "./base.json",',
+ ' "compilerOptions": {',
+ ' // maps siblings to their packages/*/src trees',
+ ' "outDir": "dist"',
+ ' },',
+ ' "exclude": ["**/*.test.ts"]',
+ '}',
+ '',
+ ].join('\n'),
+ );
+
+ expect(parseTsconfig(path.join(dir, 'tsconfig.json')).compilerOptions?.outDir).toBe('dist');
+ expect(effectiveNoEmit(path.join(dir, 'tsconfig.json'))).toBe(true);
+ });
+
+ it('still THROWS on a config nobody can read, which is what the catch is for', () => {
+ // `effectiveNoEmit` is written against this contract: a genuinely unreadable
+ // config widens the scan rather than dropping a package out of it. The fix
+ // narrows what counts as unreadable; it does not remove the margin.
+ const dir = tmpdir();
+ fs.writeFileSync(path.join(dir, 'tsconfig.json'), '{ "compilerOptions": }\n');
+ expect(() => readTsconfig(path.join(dir, 'tsconfig.json'))).toThrow();
+ expect(effectiveNoEmit(path.join(dir, 'tsconfig.json'))).toBe(false);
+ });
+});
+
+describe('the scope this gate ratchets, now that the emit question is answered', () => {
+ // Fixing the parse moves the specifier leg's membership exactly once, and the
+ // two packages below are both halves of that move. Pinned here so the change
+ // is a decision on the record rather than a side effect nobody measured — the
+ // failure objectui#5367 was filed to end.
+
+ it('reads `@object-ui/fields` as NOT specifier-preserving, so it leaves the leg', () => {
+ // Its `tsc` step inherits the root's `noEmit: true` and only type-checks;
+ // `dist` is written by vite-plugin-dts, which resolves relative specifiers
+ // while bundling. Measured at the time this landed: 0 extensionless of 155
+ // relative specifiers in its built `dist`, and 0 extensionless in its
+ // sources — so nothing live left the leg with it. The sources keep a
+ // STRICTER guard than this heuristic: that config pins `nodenext`, under
+ // which a missing relative extension is TS2835 in the package's own build.
+ const dir = path.join(REPO_ROOT, 'packages/fields');
+ const build = JSON.parse(fs.readFileSync(path.join(dir, 'package.json'), 'utf8')).scripts.build;
+
+ expect(effectiveNoEmit(path.join(dir, 'tsconfig.json'))).toBe(true);
+ expect(buildPreservesSpecifiers(build, dir)).toBe(false);
+ expect(parseTsconfig(path.join(dir, 'tsconfig.json')).compilerOptions?.noEmit).toBeUndefined();
+ });
+
+ it('keeps `@object-ui/auth` in the leg on its DECLARED noEmit, not on a catch', () => {
+ // Its config throws under the old reader too, so it was in the leg by
+ // accident. It declares `"noEmit": false` outright, so the verdict is
+ // unchanged — what changes is that it is now a reading.
+ const dir = path.join(REPO_ROOT, 'packages/auth');
+ const build = JSON.parse(fs.readFileSync(path.join(dir, 'package.json'), 'utf8')).scripts.build;
+
+ expect(parseTsconfig(path.join(dir, 'tsconfig.json')).compilerOptions?.noEmit).toBe(false);
+ expect(effectiveNoEmit(path.join(dir, 'tsconfig.json'))).toBe(false);
+ expect(buildPreservesSpecifiers(build, dir)).toBe(true);
+ });
+});
+
 describe('the load leg evaluates rather than resolves', () => {
  it('fails a module that RESOLVES but throws on evaluation', () => {
  // The card's sharpest measured point: plugin-charts' entry resolved fine and

scripts/check-node-esm-load.mjs

@@ -87,15 +87,19 @@
  * specifiers seen by the mask, 2133 by the parser, the missing one a real
  * `import`. Leg 1 now reads the file as written and asks the shared
  * TypeScript scanner what the module edges are (objectui#5382); see
- * `relativeSpecifiers`. `readTsconfig` below is the same class, still open
- * as objectui#5367.
+ * `relativeSpecifiers`. `readTsconfig` below was the same class in the same
+ * file — three regexes over a tsconfig's raw text, blind to JSON strings,
+ * throwing on 61 of the repository's 91 configs — and took the same remedy
+ * under objectui#5367: TypeScript's own tsconfig reader.
  */
 
 import { execFileSync, spawnSync } from 'node:child_process';
 import { existsSync, readdirSync, readFileSync, statSync } from 'node:fs';
 import { dirname, join, resolve } from 'node:path';
 import { fileURLToPath } from 'node:url';
 
+import ts from 'typescript';
+
 import { SKIP_DIRS, TOOLING_FILE, discoverPackages, moduleSpecifiers } from './check-phantom-dependencies.mjs';
 
 const scriptDir = dirname(fileURLToPath(import.meta.url));
@@ -120,13 +124,52 @@ export const MIN_LOADED = 3;
 
 // ── which builds preserve specifiers ─────────────────────────────────────────
 
-/** Parse a tsconfig, tolerating the comments this repository writes in them. */
+/**
+ * Parse a tsconfig, tolerating the comments this repository writes in them.
+ *
+ * ## Why this asks TypeScript rather than blanking comments (objectui#5367)
+ *
+ * It used to be three ordered `replace` calls over the raw text — blank block
+ * comments, blank whole-line comments, drop trailing commas — feeding
+ * `JSON.parse`. None of them knew what a JSON string was, so a slash-star
+ * sequence 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.
+ *
+ * That is not exotic: `"@/*"` in a `paths` map and `"**\/*.test.ts"` in an
+ * `exclude` list are the two commonest lines in this repository's tsconfigs,
+ * and they are a matched opener and closer. Measured on `main` at ed35c23bb
+ * over the 91 tsconfigs tracked by git: **61 of them threw**, not the two the
+ * card was filed for. Every one of those throws was absorbed by
+ * `effectiveNoEmit`'s catch, which grades an unreadable config as emitting — so
+ * the gate's scope was being decided by a fallback rather than by a reading,
+ * with no signal that it had happened.
+ *
+ * This is the same class as the specifier mask objectui#5382 retired further
+ * down, and it takes the same remedy: ask the parser instead of pattern-
+ * matching around it. `ts.parseConfigFileTextToJson` is 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 one
+ * `tsc` itself would compute. Re-measured with it: **0 of 91 throw**.
+ *
+ * No new dependency edge: `typescript` is a root devDependency and this module
+ * already loads it on every run, transitively through the shared scanner it
+ * imports from `check-phantom-dependencies.mjs`.
+ *
+ * Still THROWS on a genuinely unparseable config, deliberately — that is the
+ * contract `effectiveNoEmit`'s conservative catch is written against, and the
+ * contract `scripts/__tests__/check-node-esm-load.test.ts` asserts over every
+ * tsconfig in the repository so this class cannot come back silently.
+ *
+ * @param {string} path absolute path to a tsconfig
+ * @returns {Record<string, unknown>} the parsed config (`{}` for an empty file,
+ * which is what TypeScript reads it as: inherit everything)
+ */
 export function readTsconfig(path) {
- const raw = readFileSync(path, 'utf8')
- .replace(/\/\*[\s\S]*?\*\//g, '')
-.replace(/^[ \t]*\/\/.*$/gm, '')
- .replace(/,(\s*[}\]])/g, '$1');
- return JSON.parse(raw);
+ const parsed = ts.parseConfigFileTextToJson(path, readFileSync(path, 'utf8'));
+if (parsed.error) {
+throw new Error(`${path}: ${ts.flattenDiagnosticMessageText(parsed.error.messageText, ' ')}`);
+}
+ return parsed.config ?? {};
 }
 
 /**
@@ -146,6 +189,13 @@ export function effectiveNoEmit(configPath, seen = new Set()) {
  } catch {
  // An unreadable config is graded as emitting, so a parse failure widens the
  // scan rather than silently dropping a package out of it.
+ //
+ // This is the fallback for a config nobody can read, and until objectui#5367
+ // it was the NORMAL path: the regex strip `readTsconfig` used threw on 61 of
+ // this repository's 91 tsconfigs, so most of this gate's scope came from
+ // here instead of from a reading. With a real JSONC parse the branch is
+ // reached by nothing in the repository today, which is what it was always
+ // meant to be — a margin, not the mechanism.
  return false;
  }
  if (typeof config.compilerOptions?.noEmit === 'boolean') return config.compilerOptions.noEmit;
@@ -176,10 +226,38 @@ export function effectiveNoEmit(configPath, seen = new Set()) {
  * the artifact comes from the two steps after it. Grading it on the command
  * alone reported 124 findings against a package that emits none of them.
  *
- * A pipeline that runs an EMITTING `tsc` and a bundler (`@object-ui/fields`)
- * counts as preserving: the `tsc` half still writes files, and judging it is the
- * conservative direction — a false positive lands in the ledger with a reason, a
- * false negative ships.
+ * A pipeline that runs an EMITTING `tsc` and a bundler counts as preserving: the
+ * `tsc` half still writes files, and judging it is the conservative direction —
+ * a false positive lands in the ledger with a reason, a false negative ships.
+ * No published package matches that shape today; the rule is kept because the
+ * shape is legal, not because something wears it.
+ *
+ * ## `@object-ui/fields` used to be this paragraph's example, and it was wrong
+ *
+ * It was named here as the emitting-tsc-plus-bundler case, and it is the
+ * opposite: `packages/fields/tsconfig.json` inherits the root's `noEmit: true`
+ * and its own comment says so in as many words — `tsc` CHECKS, `dist` is written
+ * by vite-plugin-dts. The example survived only because `readTsconfig` could not
+ * parse that file at all (objectui#5367): the throw hit `effectiveNoEmit`'s
+ * catch, the catch answered "emitting", and the wrong answer got written down as
+ * a fact about the package.
+ *
+ * Fixing the parse therefore MOVES this gate's scope, once, in one place:
+ * `@object-ui/fields` leaves the specifier leg, 13 published specifier-
+ * preserving ESM packages become 12. It is the only membership change in the
+ * repository — `@object-ui/auth`'s config throws today too, but it declares
+ * `"noEmit": false` outright, so its verdict is unchanged and merely stops being
+ * an accident.
+ *
+ * Letting it drop is deliberate, and it costs no coverage. Measured at ed35c23bb:
+ * `fields`' sources carry 0 extensionless relative specifiers, so no live finding
+ * leaves with it; its built `dist` carries 0 extensionless out of 155 relative
+ * specifiers, because rolldown resolves them while bundling, so the property this
+ * leg ratchets is not observable in what `fields` publishes. And the sources keep
+ * a STRICTER guard than this heuristic: that tsconfig pins `"moduleResolution":
+ * "nodenext"`, under which a missing relative extension is TS2835 and a bare
+ * directory import is TS2834 — the compiler rejects in `fields`' own build step
+ * what this leg would only have reported.
  *
  * @param {string} buildScript the package's `scripts.build`, or ''
  * @param {string} pkgDir absolute path to the package, for resolving `-p`
@@ -400,9 +478,10 @@ export const ESM_MODULE_EDGE = new Set(['import', 'export', 'dynamic import()',
  * comment", and the two `replace` calls whose ORDER decided the answer were the
  * defect. Asking the parser removes the whole class at once: comments, strings,
  * template literals and regex literals stop being questions this gate has an
- * opinion about. `readTsconfig()` above still strips comments with regexes and
- * is a SEPARATE live instance of the same class — objectui#5367, deliberately
- * not touched here.
+ * opinion about. `readTsconfig()` above was a SEPARATE live instance of the same
+ * class — regexes over a tsconfig's raw text, deliberately left for its own card
+ * — and it is closed the same way, by TypeScript's own tsconfig reader
+ * (objectui#5367).
  *
  * ## Why the sibling gate's scanner rather than a second parser
  *