-
Notifications
You must be signed in to change notification settings - Fork 171
feat: add JSONC support across SDK and plugins #4312
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
- Implemented JSONC (JSON with comments) support in the SDK and various plugins, allowing for comments and trailing commas in JSON files. - Updated `loadProjectFromDirectory.ts` to use `jsonc-parser` for reading settings files. - Added `jsonc-parser` dependency to relevant packages. - Modified plugins to support JSONC format. - Ensured backward compatibility with existing JSON files.
🦋 Changeset detectedLatest commit: 7f61116 The changes in this PR will be included in the next version bump. This PR includes changesets to release 8 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 12f93cbbd5
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
|
In the tests are accusing error on something unrelated to this task, I will try to fix it but was this happening before the changes too? |
|
I have read the CLA Document and I hereby sign the CLA |
|
Hi @darklight9811, Thanks for the PR. Unfortunately, I can't merge this. JSON with comments is not roundtrip-safe. A JSON with comments can be parsed buy it can't be serialized again. Applications like Sherlock and Fink that modify the settings.json file through the UI will drop all comments. The problem is similar to #1143 I recommend to open an issue first next time to gauge if a PR would be accepted. |
This pull request updates several packages to use the
jsonc-parserlibrary for parsing JSON files that may include comments or trailing commas, improving robustness when handling configuration and settings files across the codebase. The changes affect multiple plugins and the core SDK, and also update dependency listings to ensurejsonc-parseris available where needed.Core improvements to JSON parsing:
Replaced all instances of
JSON.parsewithparseJsonc(fromjsonc-parser) in project loading and plugin import/export logic, allowing for more flexible and error-tolerant configuration file parsing. [1] [2] [3] [4] [5] [6] [7]Updated imports to include
parseJsoncfromjsonc-parserin all affected files, ensuring the new parser is used throughout. [1] [2] [3] [4]Dependency and package management:
Added
jsonc-parseras a dependency inpackage.jsonfiles for@inlang/sdk,@inlang/plugins/icu1, and@inlang/plugins/i18next, ensuring it is available wherever needed. [1] [2] [3]Updated
pnpm-lock.yamlto reflect the addition ofjsonc-parserand to update some indirect dependencies, especially around Vitest and Vite, to newer versions. [1] [2] [3] [4] [5] [6] [7] [8] [9] [10] [11] [12]These changes enhance the resilience of the project against malformed JSON and make it easier to work with user-edited configuration files that may not strictly adhere to the JSON standard.