Repository navigation
fix: allow trailing commas when parsing JSONC - #295
naveentehrpariya wants to merge 1 commit into
Conversation
confbox 0.3.0 made parseJSONC reject trailing commas unless
allowTrailingComma is passed, and pkg-types never opted in, so reading
a JSONC file with a trailing comma throws:
// tsconfig.json — accepted by tsc and VS Code
{
"compilerOptions": {
"strict": true,
},
}
await readTSConfig('tsconfig.json')
// SyntaxError: Expected double-quoted property name in JSON
This also broke two of the repo's own tests, which went unnoticed
because the test script only runs lint and typecheck:
- 'read package.json (jsonc)'
- 'reads package.json with comments (JSONC)'
Both use test/fixture/jsonc/package.json, which has a trailing comma,
so the suite already asserts trailing commas must parse.
Pass { allowTrailingComma: true } at the three parseJSONC call sites.
|
No actionable comments were generated in the recent review.
 |
| Layer / File(s) | Summary |
|---|---|
Allow trailing commas in configuration parsing src/packagejson/utils.ts, src/tsconfig/utils.ts |
The JSONC fallbacks in readPackage and readPackageJSON now allow trailing commas. readTSConfig also allows trailing commas. |
Priority:
Estimated code review effort: 2 (Simple) | ~10 minutes
Change: Bug fix
Suggested reviewers: pi0
Merge Risk: 
91fee
-
No actionable merge-blocking risk was identified in this focused parsing change.
Pre-merge checks: 
-
Check name Status Explanation Description Check Check skipped - CodeRabbit’s high-level summary is enabled. Title check The title names the main change: trailing commas are now allowed when the JSONC parser runs. The raw summary confirms that readPackage, readPackageJSON, and readTSConfig now accept trailing commas in … Docstring Coverage Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat… Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request. Full details: Title check
-
Explanation
The title names the main change: trailing commas are now allowed when the JSONC parser runs. The raw summary confirms that readPackage, readPackageJSON, and readTSConfig now accept trailing commas in their JSONC fallback paths. The title uses the conventional fix: prefix and stays under one line.
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 3 functions across 2 files.
-
Finishing Touches
-
🧪 Generate unit tests (beta)
-
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Comment @coderabbitai help to get the list of available commands.
Problem
confbox 0.3.0 made
parseJSONCreject trailing commas unlessallowTrailingCommais passed (it strips comments, then hands the text to nativeJSON.parse). pkg-types calls it without options at three sites, so reading a JSONC file with a trailing comma throws:This already breaks two of the repo's own tests
On a fresh clone of
main,vitest runfails:test/index.test.ts› read package.json (jsonc)test/index.test.ts› reads package.json with comments (JSONC)Both read
test/fixture/jsonc/package.json, which has a trailing comma, so the suite already asserts that trailing commas must parse. It went unnoticed because"test": "pnpm lint && pnpm typecheck"andchecks.ymlnever run vitest.Bisects to the
confbox^0.2.4→^0.3.0bump: 0.1.8 and 0.2.4 tolerate trailing commas, 0.3.x throws without the opt-in. The confbox default change was intentional and documented as breaking, so the gap is pkg-types not opting in.Fix
Pass
{ allowTrailingComma: true }at the threeparseJSONCcall sites (src/tsconfig/utils.ts,src/packagejson/utils.ts×2). This matches what pkg-types did before migrating to confbox, and what TypeScript's own tsconfig parser accepts.After this change
vitest rungoes from 3 failures to 1. The remaining failure (detects Deno workspace) is pre-existing and unrelated —findWorkspaceDirdefaultsworkspaceFileto"furthest"and the repo root now has its ownpnpm-workspace.yaml, so the fixture resolves to the repo root. Happy to open that separately.tsc --noEmit,oxlintandoxfmt --checkare all clean.Might also be worth running
vitestinchecks.ymlso this class of regression surfaces in CI.Summary by CodeRabbit