feat(common-config): Add autofix for sort-keys (resolves #17).
#18
Conversation
WalkthroughThe ESLint configuration was updated to include the Changes
Possibly related issues
Tip ⚡️ Faster reviews with caching
Enjoy the performance boost—your workflow just got faster. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🔇 Additional comments (2)
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
CommonConfig.mjs(3 hunks)package.json(1 hunks)
🔇 Additional comments (3)
package.json (1)
29-29: Add new peer dependency for eslint-plugin-sort-keys-plus
The entry"eslint-plugin-sort-keys-plus": "^1.4.0"correctly matches the plugin integration inCommonConfig.mjs. Ensure that this version is published and aligns with your desired feature set.CommonConfig.mjs (2)
8-8: Import the SortKeysPlusPlugin
You’ve correctly addedimport SortKeysPlusPlugin from "eslint-plugin-sort-keys-plus";. Verify that the import path matches the peer dependency inpackage.jsonto avoid resolution errors.
22-22: Register the sort-keys-plus plugin
The line"sort-keys-plus": SortKeysPlusPlugin,under thepluginssection is in the right spot and follows the existing pattern.
|
for the lint errors in the Stylistic rules, I believe suppressing the newly added rule (instead of sort-keys) would remove the warnings when running eslint sometimes we do suppress the rule because alphabetic ordering could make the code harder to read (, though for the one's specifically in the StylisticConfig, it's not like we can't seperate them until groups.) @hoophalab could you review this and provide suggestions to the above rule conflict? |
There was a problem hiding this comment.
Great job! sort-keys-plus works exactly as expected!
For @junhaoliao 's info: replacing sort-keys with sort-keys-plus doesn't change the result of lint:fix in yscope-log-viewer, except that we need to rename this //eslint-disable line.
diff --git a/src/components/theme.tsx b/src/components/theme.tsx
@@ -71,13 +71,13 @@ const APP_THEME = extendTheme({
body: "var(--ylv-ui-font-family)",
},
radius: {
- /* eslint-disable sort-keys */
+ /* eslint-disable sort-keys-plus/sort-keys */
xs: "2px",
sm: "2px",
md: "2px",
lg: "2px",
xl: "2px",
- /* eslint-enable sort-keys */
+ /* eslint-enable sort-keys-plus/sort-keys */
},
});
@junhaoliao can give a final review after the comments are addressed.
i agree that we can suppress @zzxthehappiest Could you also check whether the following repos are currently suppressing the
|
|
One more comment: Running |
I confirm the line I mentioned was the only line in |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
CommonConfig.mjs (1)
361-361: Disable the core sort-keys rule
This correctly turns off the built-insort-keysrule to prevent duplicate warnings now thatsort-keys-plus/sort-keysis in use.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (1)
CommonConfig.mjs(3 hunks)
🔇 Additional comments (2)
CommonConfig.mjs (2)
22-22: Register the new plugin correctly
The addition of"sort-keys-plus": SortKeysPlusPluginto thepluginssection is correct and aligns with the import.
362-366: Configure the sort-keys-plus rule
The new"sort-keys-plus/sort-keys"rule is set up correctly with your desired options (allowLineSeparatedGroups,caseSensitive,minKeys,natural).
| @@ -5,6 +5,7 @@ import ImportNewlinesPlugin from "eslint-plugin-import-newlines"; | |||
| import JsdocPlugin from "eslint-plugin-jsdoc"; | |||
| import NoAutofixPlugin from "eslint-plugin-no-autofix"; | |||
| import SimpleImportSortPlugin from "eslint-plugin-simple-import-sort"; | |||
| import SortKeysPlusPlugin from "eslint-plugin-sort-keys-plus"; | |||
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify plugin installation, lockfile, and documentation
Ensure that eslint-plugin-sort-keys-plus is declared in your package.json (dependencies or peerDependencies) and that you have committed the updated lockfile (package-lock.json or yarn.lock). Without this, ESLint will report a missing plugin error. Additionally, consider updating your README or other documentation to mention the new plugin and its configuration options.
You can verify installation with:
🏁 Script executed:
grep -R '"eslint-plugin-sort-keys-plus"' package.jsonLength of output: 101
🏁 Script executed:
#!/bin/bash
echo "Checking package-lock.json:"
if [ -f package-lock.json ]; then
grep -R "eslint-plugin-sort-keys-plus" package-lock.json
else
echo "package-lock.json not found"
fi
echo "Checking yarn.lock:"
if [ -f yarn.lock ]; then
grep -R "eslint-plugin-sort-keys-plus" yarn.lock
else
echo "yarn.lock not found"
fiLength of output: 492
🏁 Script executed:
#!/bin/bash
echo "Searching Markdown files for plugin mention:"
grep -R "eslint-plugin-sort-keys-plus" --include="*.md" .Length of output: 156
Update documentation for eslint-plugin-sort-keys-plus
I’ve confirmed that eslint-plugin-sort-keys-plus is declared in your package.json and present in package-lock.json. However, I didn’t find any references in your Markdown documentation. Please update your README (or other docs) to mention the new plugin and provide its configuration options.
• File to update: README.md (or relevant docs) – add a section on ESLint plugins, include sort-keys-plus and its config.
For this one, the warnings I mentioned are those reported by running command |
Thanks @hoophalab 's for checking, there is only one place suppressing |
|
@zzxthehappiest Thanks for checking and confirming! Let's do these:
|
Fixed! |
Description
This PR is to add support for autofix of
sort-keys. (#17 )It seems "sort-keys-fix" only accept
caseSentiveandnaturalvalues, there is noallowLineSeparatedGroupsandminKeys, and this project seems not actively maintained. I found this fork: https://www.npmjs.com/package/eslint-plugin-sort-keys-plus which supports theallowLineSeparatedGroupsandminKeys.Checklist
breaking change.
Validation performed
Run
eslint .at the root directory of this project, and found 13 warnings (before useeslint-plugin-sort-keys-plusthere were much more warnings because it didn't allow line separated groups and minimum keys) and 1 error, and all warnings were related to sort issues:It is surprising that the lint project itself is not well linted -_-||
And then I use WebStorm to check these warnings, it can show the fix option with yellow warning like:
Summary by CodeRabbit