feat(pkg): add option packageSortOrderPreset
#475
Conversation
🦋 Changeset detectedLatest commit: 0db438d 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 configuration option, Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Prettier
participant Plugin
participant Sorter
User->>Prettier: Format package.json with plugin
Prettier->>Plugin: Pass options (including packageSortOrderPreset)
Plugin->>Sorter: Call sort() with selected preset
Sorter->>Plugin: Return sorted properties
Plugin->>Prettier: Return formatted package.json
Prettier->>User: Output formatted package.json
Possibly related PRs
Suggested labels
Poem
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: |
|
Looks like you need to configure |
There was a problem hiding this comment.
Caution
Changes requested ❌
Reviewed everything up to 4af67ae in 1 minute and 55 seconds. Click for details.
- Reviewed
836lines of code in11files - Skipped
0files when reviewing. - Skipped posting
9draft 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. .changeset/silver-shirts-relax.md:5
- Draft comment:
Ensure the changelog clearly notes that the default preset has changed from 'unts' to 'npm', as this is a breaking change. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%The comment is asking for a changelog update, which is similar to asking for a PR description update. This violates the rule against asking the PR author to update the PR description or similar documentation.
2. packages/pkg/src/index.ts:78
- Draft comment:
Confirm that setting the default for packageSortOrderPreset to 'npm' (instead of the previous 'unts') is intentional and documented as breaking. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50% The comment makes an incorrect assumption about there being a previous default of 'unts'. This is a new option being added, not a change to an existing option. Additionally, asking authors to verify their intentions violates our rules about not asking for confirmations or explanations. Could there be some other file or commit history that shows 'unts' was previously the default? Could this be a multi-step change where the previous default was changed? Even if there was a previous default, asking for confirmation of intentional changes violates our rules. The author deliberately wrote this code and we should assume they meant what they wrote. Delete this comment as it both makes an incorrect assumption and violates our rule about not asking authors to confirm their intentions.
3. .changeset/silver-shirts-relax.md:12
- Draft comment:
Typographical suggestion: Consider replacing "compatible to" with "compatible with" for smoother reading. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50% While the suggestion is technically correct (we do say "compatible with" in English), this is a changelog file that will be committed and likely never looked at again. The meaning is perfectly clear either way. Making authors fix minor grammar in changelogs seems overly pedantic and not worth the back-and-forth. The grammar correction could improve documentation quality and professionalism. Poor grammar could reflect badly on the project. While correct grammar is good, this is an internal changelog file where the meaning is perfectly clear. The benefit doesn't justify the overhead of requesting changes. Delete this comment as it's an overly pedantic grammar suggestion on a changelog file that doesn't materially impact understanding.
4. packages/pkg/src/rules/sort.ts:114
- Draft comment:
Typo detected: the string 'binary,' includes an extra comma. Was this intended, or should it be 'binary'? - Reason this comment was not posted:
Marked as duplicate.
5. packages/pkg/src/types.ts:50
- Draft comment:
Typo: The union type for packageSortOrderPreset currently includes 'unts'. Please verify if it should be 'un-ts' or a different intended value. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50% Without more context about what these presets actually mean or represent, I cannot be certain that 'unts' is actually a typo. 'unts' could be an intentional preset name. The comment is speculative and asks for verification rather than pointing out a definitive issue. This violates the rule about not asking authors to verify things. Maybe 'unts' is actually a typo and fixing it now would save time later. The suggestion seems reasonable given that 'un-ts' looks more like a conventional package name format. Without strong evidence that 'unts' is definitely wrong, we should trust that the author knows the correct preset names. The comment is asking for verification rather than stating a definitive issue. Delete the comment because it's speculative and asks for verification rather than pointing out a definitive issue. We don't have strong evidence that 'unts' is incorrect.
6. packages/pkg/test/engines.spec.ts:18
- Draft comment:
Typo: The value 'unts' forpackageSortOrderPresetlooks like it might be a misspelling. Please verify if it should be 'un-ts' or another intended value. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50% Since this is a test file, the value 'unts' could be intentionally incorrect to test error handling. Without knowing the prettier-plugin-pkg package's valid options or the test's purpose, we can't be certain this is actually a typo. The comment is also asking for verification, which violates our rules. The value 'unts' could actually be a valid preset name that I'm not aware of. Also, since this is a test file, even an invalid value might be intentional for testing purposes. You're right - without strong evidence that 'unts' is definitely wrong, and given this is a test file where invalid values might be intentional, we shouldn't keep this comment. Delete the comment because it's speculative, asks for verification, and we don't have strong evidence that 'unts' is actually incorrect, especially in a test context.
7. packages/pkg/test/files.spec.ts:15
- Draft comment:
Typographical error: The value'unts'might be a typo. If you intended to reference the repository owner or a similar identifier, consider changing it to'un-ts'. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50% This is a test file using a prettier plugin called 'prettier-plugin-pkg'. The packageSortOrderPreset appears to be a configuration option, but without seeing the plugin's documentation, I can't be certain whether 'unts' is actually incorrect. The suggestion seems speculative - it's guessing that this might be referring to 'un-ts' but doesn't show strong evidence that this is definitely wrong. I don't have access to the plugin's documentation to verify the valid values for packageSortOrderPreset. 'unts' could be a valid preset name. Without clear documentation showing that 'unts' is invalid and 'un-ts' is correct, this comment is too speculative. Delete this comment as it makes assumptions about the correct value without strong evidence that the current value is wrong.
8. packages/pkg/test/test.spec.ts:47
- Draft comment:
The value 'unts' for the newpackageSortOrderPresetoption appears to be a typographical error. It might be intended to be 'un-ts' (matching the repository owner) or another correct preset name. Could you please verify? - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50% Since this is a test file, the value 'unts' is likely intentionally chosen for testing purposes. The consistent use in both test cases suggests it's deliberate. The comment is speculative and asks for verification, which violates our rules. We don't have evidence that 'unts' is actually wrong - it could be a valid preset name we're unaware of. Maybe 'unts' really is a typo and catching it early could save debugging time later. The suggestion of 'un-ts' seems plausible. Even if it is a typo, asking for verification violates our rules. If it's wrong, tests would fail. We should trust the author's intention in test files. Delete the comment. It's speculative, asks for verification, and we lack strong evidence that 'unts' is actually incorrect.
9. packages/pkg/test/test.spec.ts:60
- Draft comment:
Typo alert: The string literal 'unts' may be a typographical error. Should it be 'un-ts' to match the expected naming or is this intended? Please verify. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50% Since this is a test file, the value 'unts' is likely intentional as it's used consistently in multiple test cases. Without access to the prettier-plugin-pkg documentation or implementation, we can't know if 'unts' is actually a valid preset name. The comment is speculative and asks for verification rather than pointing out a definite issue. I might be wrong if 'unts' is genuinely a typo that would cause the tests to fail. The consistency of the typo across multiple tests doesn't necessarily mean it's correct. However, this is exactly the kind of speculative comment we should avoid - it's asking for verification without strong evidence of an actual issue. If 'unts' were invalid, the tests would fail. Delete the comment as it violates the rules against speculative comments and asking for verification without clear evidence of an issue.
Workflow ID: wflow_is3bzFQkaezebC5D
You can customize 
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
packages/pkg/src/types.ts (1)
50-50: Well-defined type with proper constraints.The new
packageSortOrderPresetproperty is correctly typed with string literal unions and marked as optional for backward compatibility.Consider adding JSDoc comments to document what each preset represents:
export interface FormatOptions { packageSortOrder?: string[] packageIgnoreSort?: string[] + /** + * Preset for sorting package.json properties + * - 'npm': Official npm documentation order + * - 'npm-plus': Compatible with prettier-plugin-packagejson + * - 'unts': Previous default sorting order + */ packageSortOrderPreset?: 'npm-plus' | 'npm' | 'unts' }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (3)
packages/pkg/test/__snapshots__/ignore-sort.spec.ts.snapis excluded by!**/*.snappackages/pkg/test/__snapshots__/sort-order-preset.spec.ts.snapis excluded by!**/*.snappackages/pkg/test/__snapshots__/sort-order.spec.ts.snapis excluded by!**/*.snap
📒 Files selected for processing (8)
.changeset/silver-shirts-relax.md(1 hunks)packages/pkg/src/index.ts(1 hunks)packages/pkg/src/rules/sort.ts(3 hunks)packages/pkg/src/types.ts(1 hunks)packages/pkg/test/engines.spec.ts(1 hunks)packages/pkg/test/files.spec.ts(1 hunks)packages/pkg/test/sort-order-preset.spec.ts(1 hunks)packages/pkg/test/test.spec.ts(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Codacy Static Code Analysis
🔇 Additional comments (11)
packages/pkg/test/files.spec.ts (1)
15-15:✅ Verification successful
LGTM! Backward compatibility maintained.
The addition of
packageSortOrderPreset: 'unts'correctly maintains the previous default sorting behavior for this test.However, ensure comprehensive test coverage exists for all three presets (
npm,npm-plus,unts). Run this script to verify:
🏁 Script executed:
#!/bin/bash # Description: Check if tests exist for all packageSortOrderPreset values # Expected: Should find tests for 'npm', 'npm-plus', and 'unts' presets echo "Searching for tests with packageSortOrderPreset option..." rg -A 2 -B 2 "packageSortOrderPreset.*:" --type ts echo -e "\nChecking for specific preset values in tests..." for preset in "npm" "npm-plus" "unts"; do echo "Testing preset '$preset':" rg "packageSortOrderPreset.*['\"]$preset['\"]" --type ts doneLength of output: 3657
Ensure Comprehensive Preset Test Coverage
All three
packageSortOrderPresetvalues are indeed tested:
- Dynamic tests in packages/pkg/test/sort-order-preset.spec.ts iterate over
['npm', 'npm-plus', 'unts'].- Specific unit tests for
'unts'appear in files.spec.ts, engines.spec.ts, and test.spec.ts.- The default
'npm'preset is verified via the dynamic suite, and'npm-plus'is covered there as well.No further action is required. The explicit
packageSortOrderPreset: 'unts'in files.spec.ts is appropriate, and all presets have dedicated tests.packages/pkg/test/engines.spec.ts (1)
18-18: LGTM! Consistent with backward compatibility approach.The addition of
packageSortOrderPreset: 'unts'is consistent with other test files and maintains the previous default sorting behavior.packages/pkg/test/test.spec.ts (2)
47-47: LGTM! Appropriate option addition.The
packageSortOrderPreset: 'unts'option is correctly added to maintain the previous default sorting behavior in the randomize test.
60-60: LGTM! Consistent test configuration.The addition of
packageSortOrderPreset: 'unts'to the preprocess test maintains consistency with other test files and preserves the expected sorting behavior.packages/pkg/test/sort-order-preset.spec.ts (1)
1-26: LGTM! Comprehensive test coverage for the new preset feature.The parameterized test approach efficiently covers all combinations of presets and fixtures using snapshot testing. This ensures that each preset produces the expected sorting behavior.
.changeset/silver-shirts-relax.md (1)
5-13: Well-documented feature with clear preset descriptions.The documentation clearly explains the three presets and provides helpful links to their specifications.
packages/pkg/src/index.ts (1)
78-107: Well-structured option configuration following Prettier conventions.The new
packageSortOrderPresetoption is properly defined with:
- Clear choice type with descriptive values
- Helpful descriptions linking to sorting specifications
- Appropriate default value ('npm')
- Consistent structure with existing options
packages/pkg/src/rules/sort.ts (4)
13-13: Good explicit typing for exported constant.Adding the
readonly string[]type annotation improves type safety and makes the API contract clear.
25-63: Well-documented NPM preset with official specification reference.The NPM preset follows the official npm documentation structure with clear reference links.
178-262: Well-structured UNTS preset with logical grouping.The UNTS preset is well-organized with clear comments grouping related properties logically.
268-300: Solid sorting logic with proper user preference handling.The implementation correctly:
- Selects the appropriate preset based on
packageSortOrderPreset- Merges user-provided sort order with preset defaults
- Maintains user preference priority through order in the merge
- Uses proper deduplication with
uniqueArray
4af67ae to
49847fc
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #475 +/- ##
==========================================
+ Coverage 85.78% 86.69% +0.91%
==========================================
Files 10 10
Lines 197 203 +6
Branches 55 57 +2
==========================================
+ Hits 169 176 +7
+ Misses 27 26 -1
Partials 1 1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
.changeset/silver-shirts-relax.md (1)
1-3:⚠️ Potential issueInconsistent version type for breaking change.
The changeset specifies "patch" but the commit message indicates a breaking change with
feat(pkg)!. This contradicts the previous review feedback that suggested it should be "major" for breaking changes according to semantic versioning.Please clarify the intended version impact and update accordingly:
- If this is truly a breaking change, use "major"
- If this is non-breaking, remove the "!" from the commit message and keep "patch"
🧹 Nitpick comments (1)
packages/pkg/README.md (1)
164-167: Comprehensive documentation with minor formatting suggestion.The documentation clearly explains the new sorting options and presets. The descriptions are accurate and helpful.
Consider adjusting the punctuation for better readability:
-You can also use the `packageSortOrder` option to specify a custom sort order, or use the `packageSortOrderPreset` option to use a preset sort order: +You can also use the `packageSortOrder` option to specify a custom sort order, or use the `packageSortOrderPreset` option to use a preset sort order. -- `npm`: sorts by [`npm`'s document](https://docs.npmjs.com/cli/v11/configuring-npm/package-json) -- `npm-plus`: sorts by [`sort-package-json`](https://github.com/keithamus/sort-package-json/blob/aa6774ad937feb83178c8bc981f08305e1d22b5c/defaultRules.md) and therefore is compatible to [`prettier-plugin-packagejson`](https://github.com/matzkoh/prettier-plugin-packagejson) +- `npm`: sorts by [`npm`'s document](https://docs.npmjs.com/cli/v11/configuring-npm/package-json) +- `npm-plus`: sorts by [`sort-package-json`](https://github.com/keithamus/sort-package-json/blob/aa6774ad937feb83178c8bc981f08305e1d22b5c/defaultRules.md) and therefore is compatible to [`prettier-plugin-packagejson`](https://github.com/matzkoh/prettier-plugin-packagejson)🧰 Tools
🪛 LanguageTool
[uncategorized] ~166-~166: Loose punctuation mark.
Context: ...ion to use a preset sort order: -npm: sorts by [npm's document](https://doc...(UNLIKELY_OPENING_PUNCTUATION)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
packages/pkg/test/__snapshots__/sort-order-preset.spec.ts.snapis excluded by!**/*.snap
📒 Files selected for processing (7)
.changeset/silver-shirts-relax.md(1 hunks).size-limit.json(1 hunks)packages/pkg/README.md(2 hunks)packages/pkg/src/index.ts(1 hunks)packages/pkg/src/rules/sort.ts(3 hunks)packages/pkg/src/types.ts(1 hunks)packages/pkg/test/sort-order-preset.spec.ts(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- .size-limit.json
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/pkg/src/types.ts
- packages/pkg/test/sort-order-preset.spec.ts
- packages/pkg/src/index.ts
🧰 Additional context used
🧬 Code Graph Analysis (1)
packages/pkg/src/rules/sort.ts (1)
packages/pkg/src/types.ts (2)
ObjectProperty(8-13)FormatOptions(47-51)
🪛 LanguageTool
.changeset/silver-shirts-relax.md
[uncategorized] ~11-~11: Loose punctuation mark.
Context: ...presets are: npm, npm-plus: - npm: sorts by [npm's document](https://doc...
(UNLIKELY_OPENING_PUNCTUATION)
packages/pkg/README.md
[uncategorized] ~166-~166: Loose punctuation mark.
Context: ...ion to use a preset sort order: - npm: sorts by [npm's document](https://doc...
(UNLIKELY_OPENING_PUNCTUATION)
⏰ Context from checks skipped due to timeout of 90000ms (5)
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: Lint and Test with Node.js 24 on windows-latest
- GitHub Check: Lint and Test with Node.js 22 on windows-latest
- GitHub Check: Lint and Test with Node.js 20 on windows-latest
- GitHub Check: Lint and Test with Node.js 18 on windows-latest
🔇 Additional comments (7)
packages/pkg/src/rules/sort.ts (5)
13-13: Good type safety improvement.Adding explicit typing to
dependencyNamesasreadonly string[]improves type safety and prevents accidental mutations.
25-66: Well-documented npm preset implementation.The NPM_SORTS array follows the official npm documentation structure and provides a solid foundation for the npm preset. The reference comment is clear and helpful.
68-174: Comprehensive npm-plus preset with proper attribution.The NPM_PLUS_SORTS array is extensive and well-aligned with the sort-package-json project. The reference documentation is accurate and helpful for users who want compatibility with prettier-plugin-packagejson.
269-272: Clean preset mapping implementation.The DEFAULT_SORT_ORDERS object provides a clean way to map preset names to their corresponding sort arrays. The structure is extensible for future presets.
278-290: Robust sorting logic with proper fallback.The updated sort function correctly handles the new packageSortOrderPreset option with proper fallback behavior. The logic ensures that user-defined sort order takes precedence, followed by preset order, and finally the primary order. The uniqueArray function prevents duplicates.
.changeset/silver-shirts-relax.md (1)
5-12: Clear and comprehensive feature description.The changeset description effectively explains the new
packageSortOrderPresetoption, its available presets, and their purposes. The documentation links provide helpful context for users.🧰 Tools
🪛 LanguageTool
[uncategorized] ~11-~11: Loose punctuation mark.
Context: ...presets are:npm,npm-plus: -npm: sorts by [npm's document](https://doc...(UNLIKELY_OPENING_PUNCTUATION)
packages/pkg/README.md (1)
184-185: Clear and accurate interface documentation.The FormatOptions interface documentation correctly describes the new
packageSortOrderPresetoption with its allowed values.
0db438d to
6ef22e4
Compare
|
That's why the original implementation won't be accepted. 😂 |
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/workflows/size-limit.yml(0 hunks).github/workflows/vercel.yml(1 hunks)
💤 Files with no reviewable changes (1)
- .github/workflows/size-limit.yml
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Codacy Static Code Analysis
packageSortOrderPresetpackageSortOrderPreset
references salsita/node-pg-migrate#1432 (comment)
Important
Add
packageSortOrderPresetoption toprettier-plugin-pkgfor sortingpackage.jsonwithnpm,npm-plus, anduntspresets.packageSortOrderPresetoption inindex.tsto specify sorting presets forpackage.json.npm,npm-plus, anduntspresets, withnpmas the default.sort.tsusingNPM_SORTS,NPM_PLUS_SORTS, andUNTS_SORTarrays.sort()function to usepackageSortOrderPresetfor determining sort order.sort-order-preset.spec.tsto test different presets.engines.spec.ts,files.spec.ts, andtest.spec.tsto usepackageSortOrderPreset.FormatOptionsintypes.tsto includepackageSortOrderPreset.This description was created by
for 4af67ae. You can customize this summary. It will automatically update as commits are pushed.
Summary by CodeRabbit