-
Notifications
You must be signed in to change notification settings - Fork 441
Sort and Format JSONC Build Files #1970
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
base: main
Are you sure you want to change the base?
Conversation
src/build/sorter.ts
Outdated
@@ -0,0 +1,41 @@ | |||
import * as fs from "fs"; | |||
import commentJson from "comment-json"; |
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.
We already use jsonc-parser, should be able to use here.
The important thing is that the comments should be there, while it's all being stripped out in this PR.
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.
I think we don't need to check in a full sorter, it should be enough to check whether it's sorted and throw error if not when linting.
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.
What code did you use to sort it one-time though? Might be nice to be able to validate one-time.
src/build.ts
Outdated
@@ -396,3 +397,4 @@ async function emitDom() { | |||
} | |||
|
|||
await emitDom(); | |||
sortFiles(); |
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.
Shouldn't be part of build, it should rather be part of npm run lint
.
src/build/sorter.ts
Outdated
return foundTrailingComma; | ||
} | ||
|
||
export function sortFiles(): void { |
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.
It's not about sorting anymore, it's about verifying it's sorted. And it's not sorting "files", it's sorting fields.
src/build/sorter.ts
Outdated
} | ||
return sorted as T; | ||
} | ||
return obj; |
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.
This do not need to return an object, maybe do not return anything and throw if it's not sorted.
inputfiles/addedTypes.jsonc
Outdated
"enum": { | ||
"InsertPosition": { | ||
"name": "InsertPosition", | ||
"mixins": { |
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.
Something is definitely off here; this is indented by 2 spaces, but the rets by 4 spaces (expected).
Hello @saschanaz |
Overview
This pull request introduces a script that formats and sorts JSONC (JSON with comments) files alphabetically. The script is designed to enhance the readability and maintainability of build configuration files by ensuring that the keys within these files are consistently ordered.
Changes Made
sortObjectKeys
has been implemented to recursively sort the keys of an object. This function ensures that all nested objects are also sorted, providing a comprehensive sorting solution.sortFiles
function reads specified JSONC files, parses them while preserving comments, sorts their keys, and then writes the sorted content back to the original files.Code Explanation
The script is implemented in TypeScript and utilizes the
fs
module for file operations and thecomment-json
package to handle JSONC parsing and stringification. Below is a breakdown of the key components:sortObjectKeys Function:
sortFiles Function:
sortObjectKeys
function.Usage
To use this script, simply call the
sortFiles
function. Ensure that the specified JSONC files are located in the correct directory as indicated in the file path construction.Benefits
Next Steps
Thank you for reviewing this pull request! I look forward to your feedback.