-
-
Notifications
You must be signed in to change notification settings - Fork 31
feat(pkg): add option packageSortOrder
#468
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
🦋 Changeset detectedLatest commit: ea7d4e4 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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 |
WalkthroughA new configurable Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Prettier
participant Plugin as prettier-plugin-pkg
participant Sorter
User->>Prettier: Run Prettier with plugin and packageSortOrder option
Prettier->>Plugin: Parse package.json
Plugin->>Sorter: format(properties, options)
Sorter->>Sorter: Merge packageSortOrder with default order
Sorter->>Plugin: Return sorted properties
Plugin->>Prettier: Return formatted output
Prettier->>User: Output formatted package.json
Assessment against linked issues
Assessment against linked issues: Out-of-scope changesNo out-of-scope changes found. Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
packages/pkg/src/index.tsOops! Something went wrong! :( ESLint: 9.28.0 Error [ERR_MODULE_NOT_FOUND]: Cannot find package '@1stg/eslint-config' imported from /eslint.config.js 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (5)
✨ 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 (
|
|
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. |
prettier-plugin-autocorrect
prettier-plugin-pkg
prettier-plugin-sh
prettier-plugin-sql
prettier-plugin-toml
commit: |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #468 +/- ##
==========================================
+ Coverage 85.10% 85.41% +0.31%
==========================================
Files 10 10
Lines 188 192 +4
Branches 51 52 +1
==========================================
+ Hits 160 164 +4
Misses 27 27
Partials 1 1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
84fe0ad to
03b725a
Compare
4d6f564 to
eba22c5
Compare
JounQin
left a comment
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.
LGTM ❤️
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.
Caution
Changes requested ❌
Reviewed everything up to 7083d69 in 1 minute and 23 seconds. Click for details.
- Reviewed
210lines of code in6files - Skipped
0files when reviewing. - Skipped posting
1draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. packages/pkg/src/rules/sort.ts:121
- Draft comment:
The merging of user-provided packageSortOrder with the default primary array using a uniqueArray is effective. Consider adding an inline comment to clarify that duplicate keys are removed and the order of first occurrence is preserved. - Reason this comment was not posted:
Confidence changes required:20%<= threshold50%None
Workflow ID: wflow_e82B0A9caXALGCDO
You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.
Co-authored-by: ellipsis-dev[bot] <65095814+ellipsis-dev[bot]@users.noreply.github.com>
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.
Actionable comments posted: 1
🧹 Nitpick comments (2)
packages/pkg/test/sort-order.spec.ts (1)
7-17: Consider adding more test cases for comprehensive coverage.The test correctly verifies the basic functionality, but consider adding additional test cases to cover:
- Empty
packageSortOrderarray- Partial ordering (some keys specified, others use default)
- Invalid/non-existent keys in the sort order
- Complex nested package.json structures
packages/pkg/src/rules/sort.ts (1)
116-118: Consider inlining the helper function for simplicity.The
uniqueArrayhelper is only used once and could be inlined for simplicity, though the current approach enhances readability.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
packages/pkg/test/__snapshots__/sort-order.spec.ts.snapis excluded by!**/*.snap
📒 Files selected for processing (5)
.changeset/big-bushes-design.md(1 hunks)packages/pkg/src/index.ts(2 hunks)packages/pkg/src/rules/sort.ts(3 hunks)packages/pkg/src/types.ts(1 hunks)packages/pkg/test/sort-order.spec.ts(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Lint and Test with Node.js 22 on windows-latest
🔇 Additional comments (8)
.changeset/big-bushes-design.md (1)
1-6: LGTM! Well-documented changeset.The changeset properly documents the minor feature addition with clear package identification and description.
packages/pkg/src/types.ts (1)
47-49: LGTM! Clean type definition.The
FormatOptionsinterface is well-defined with appropriate typing for the optionalpackageSortOrderproperty. The naming convention aligns with the compatibility requirements discussed in previous reviews.packages/pkg/src/rules/sort.ts (1)
120-123: LGTM! Solid implementation of the merging logic.The function signature and merging logic are well-implemented:
- Proper destructuring of options
- Nullish coalescing handles undefined
packageSortOrder- User-specified order takes precedence over defaults
- Deduplication ensures no conflicts
packages/pkg/src/index.ts (5)
16-20: LGTM! Type imports properly support the new functionality.The addition of
FormatOptionsimport is necessary for the new options parameter, and the explicit import of existing types improves code clarity.
28-28: LGTM! Function signature correctly supports configurable options.The addition of the
optionsparameter with proper typing enables the new packageSortOrder functionality while maintaining type safety.
31-31: LGTM! Options properly passed to enable custom sorting.Passing the options to the
sortfunction is essential for implementing the packageSortOrder functionality.
46-49: LGTM! Options properly threaded through the parser.The modification correctly passes options from the parser to the format function. The type casting to
FormatOptionsis safe here since this plugin controls the interface definition.
55-66: LGTM! Plugin options properly configured following prettier conventions.The
packageSortOrderoption is correctly defined with all necessary metadata, and the default value format[{ value: [] }]follows prettier's convention for array options.
|
@JounQin IMO its now ready for merge 👍 |
|



closes #467
Important
Adds
packageSortOrderoption toprettier-plugin-pkgfor custom sorting ofpackage.jsonproperties, with tests included.packageSortOrderoption toprettier-plugin-pkginindex.tsto allow custom sorting ofpackage.jsonproperties.packageSortOrderis an array of property names, defaulting to an empty array.format()inindex.tsto usepackageSortOrderfor sorting.sort()insort.tsto incorporatepackageSortOrderand ensure unique entries.FormatOptionsinterface intypes.tsto includepackageSortOrder.sort-order.spec.tsto test custom sort order functionality.sort-order.spec.ts.snapto verify output.This description was created by
for 7083d69. You can customize this summary. It will automatically update as commits are pushed.
Summary by CodeRabbit