feat: Add support for JSON5 in configuration files #366
Conversation
|
Warning Rate limit exceeded@yamadashy has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 10 minutes and 33 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThe changes update the configuration file loader to support JSON5 syntax. The documentation in the README now details JSON5 features such as comments, trailing commas, unquoted property names, and relaxed string syntax. The configuration example has been adjusted to illustrate these JSON5 syntax features properly. Alongside these documentation updates, the JSON parsing in the configuration loader has been modified to use the JSON5 library instead of stripping comments from JSON, and error messages have been updated accordingly. A new dependency on the JSON5 library has been added to the dependency list in the package configuration. Finally, a test case has been introduced to ensure that the configuration loader successfully handles JSON5 syntax elements. Sequence Diagram(s)sequenceDiagram
participant Caller as Caller
participant Loader as Config Loader
participant FS as File System
participant JSON5 as JSON5 Parser
Caller->>Loader: Call loadAndValidateConfig(file)
Loader->>FS: Read file content (fs.readFile)
FS-->>Loader: Return file content
Loader->>JSON5: Parse content (JSON5.parse)
JSON5-->>Loader: Return parsed configuration
Loader->>Caller: Return configuration object
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Deploying repomix with
|
| Latest commit: |
e995a7d
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://1499637a.repomix.pages.dev |
| Branch Preview URL: | https://feat-config-trailing-commas.repomix.pages.dev |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #366 +/- ##
=======================================
Coverage 89.43% 89.43%
=======================================
Files 66 66
Lines 2914 2914
Branches 594 594
=======================================
Hits 2606 2606
Misses 308 308 ☔ View full report in Codecov by Sentry. |
- Replace JSON.parse with JSON5.parse for configuration file loading - Update tests to cover JSON5 features (comments, trailing commas) - Update documentation to reflect JSON5 support - Add json5 package as a dependency
yamadashy
force-pushed
the
feat/config-trailing-commas
branch
2 times, most recently
from
10a6327 to
fe82fcb
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
tests/config/configLoad.test.ts (1)
111-140: Consider expanding test coverage for JSON5 features.While the test case covers basic JSON5 features like comments, unquoted property names, and trailing commas, consider adding tests for:
- Hexadecimal numbers
- Single-quoted strings
- Error cases specific to JSON5 syntax
Apply this diff to add more test cases:
test('should parse config file with JSON5 features', async () => { const configWithJSON5Features = `{ // Output configuration output: { filePath: 'test-output.txt', style: 'plain', + hexValue: 0xFF, + singleQuoted: 'value', }, /* Ignore configuration */ ignore: { useGitignore: true, // Use .gitignore file customPatterns: [ '*.log', '*.tmp', '*.temp', // Trailing comma ], }, }`; vi.mocked(fs.readFile).mockResolvedValue(configWithJSON5Features); vi.mocked(fs.stat).mockResolvedValue({ isFile: () => true } as Stats); const result = await loadFileConfig(process.cwd(), 'test-config.json'); expect(result).toEqual({ output: { filePath: 'test-output.txt', style: 'plain', + hexValue: 255, + singleQuoted: 'value', }, ignore: { useGitignore: true, customPatterns: ['*.log', '*.tmp', '*.temp'], }, }); }); +test('should handle JSON5 syntax errors', async () => { + const invalidJSON5 = `{ + output: { + filePath: 'test-output.txt' + style: 'plain' // Missing comma + } + }`; + + vi.mocked(fs.readFile).mockResolvedValue(invalidJSON5); + vi.mocked(fs.stat).mockResolvedValue({ isFile: () => true } as Stats); + + await expect(loadFileConfig(process.cwd(), 'test-config.json')).rejects.toThrow('Invalid JSON5'); +});README.md (1)
593-598: Consider enhancing JSON5 feature documentation.While the documentation covers the key JSON5 features, consider:
- Adding links to JSON5 specification for each feature
- Providing examples for each JSON5 feature
Apply this diff to enhance the documentation:
The configuration file supports [JSON5](https://json5.org/) syntax, which allows: -Comments (both single-line and multi-line) -Trailing commas in objects and arrays -Unquoted property names -More relaxed string syntax +- Comments (both single-line and multi-line) + ```json5 + { + // Single-line comment + /* Multi-line + comment */ + } + ``` +- Trailing commas in objects and arrays + ```json5 + { + array: [1, 2, 3,], + object: { + key: 'value', + }, + } + ``` +- Unquoted property names ([spec](https://spec.json5.org/#identifiers)) + ```json5 + { + unquoted: 'value', + 'quoted-if-needed': true, + } + ``` +- More relaxed string syntax ([spec](https://spec.json5.org/#strings)) + ```json5 + { + single: 'strings', + multiline: 'can span\ + multiple\ + lines', + } + ```
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (4)
README.md(3 hunks)package.json(1 hunks)src/config/configLoad.ts(2 hunks)tests/config/configLoad.test.ts(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (9)
- GitHub Check: Test (windows-latest, 23.x)
- GitHub Check: Test (windows-latest, 22.x)
- GitHub Check: Test (windows-latest, 21.x)
- GitHub Check: Test (windows-latest, 20.x)
- GitHub Check: Test (windows-latest, 19.x)
- GitHub Check: Test (windows-latest, 18.x)
- GitHub Check: Build and run (windows-latest, 22.x)
- GitHub Check: Test (windows-latest, 18.0.0)
- GitHub Check: Cloudflare Pages
🔇 Additional comments (6)
src/config/configLoad.ts (3)
3-3: LGTM!The JSON5 import is correctly placed and follows the project's import style.
73-73: LGTM!The change to use
JSON5.parsealigns with the PR objective while maintaining backward compatibility.
78-78: LGTM!The error message is correctly updated to reflect JSON5 parsing and maintains consistency with other error messages.
package.json (1)
79-79:✅ Verification successful
Verify the json5 package version.
Let's check if there's a newer version available and if there are any security advisories.
🌐 Web query:
What is the latest version of json5?💡 Result:
The latest stable release of JSON5 is v2.2.3, released on npm with a fix to ensure it's correctly marked as the "latest" version[1][4][7]. This update addressed an issue where older versions were incorrectly tagged as latest in package managers[1].
Key details about recent versions:
- v2.2.3 (Current Latest):
Fixes npm registry tagging[1][7]- v2.2.2:
Patched a prototype pollution vulnerability related to__proto__properties[1]- v2.2.1:
Removed vulnerable minimist dependency (CVE-2021-44906)[1]The project has seen regular maintenance updates since its major v2 release in 2020, which dropped Node.js v4 support and adopted native ES5 syntax[1]. For Python implementations like pyjson5 and json-five, separate versioning exists (e.g., json-five v1.x)[3][5].
Citations:
- 1: https://github.com/json5/json5/blob/main/CHANGELOG.md
- 2: https://classic.yarnpkg.com/en/package/json5
- 3: https://pypi.org/project/json5/
- 4: https://www.npmjs.com/package/json5
- 5: https://pypi.org/project/json-five/
- 6: https://spec.json5.org
- 7: https://github.com/json5/json5/releases
- 8: https://sourceforge.net/projects/json5.mirror/
Dependency verified: json5 v2.2.3 is up-to-date.
The json5 dependency added at line 79 is using version "^2.2.3", which is confirmed as the current latest stable release with no outstanding security advisories.
README.md (2)
616-617: LGTM!The trailing comma is correctly added and maintains consistent indentation.
628-629: LGTM!The trailing commas are correctly added and maintain consistent indentation.
Also applies to: 635-636
yamadashy
force-pushed
the
feat/config-trailing-commas
branch
from
fe82fcb to
e995a7d
Compare
Closes #346
Description
This PR adds support for JSON5 in configuration files, allowing for a more flexible and developer-friendly configuration experience.
Changes
Testing
Documentation