Skip to content
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

order-properties: consider an option for sort-package-json's order #31

Closed
JoshuaKGoldberg opened this issue Sep 13, 2023 · 5 comments · Fixed by #55
Closed

order-properties: consider an option for sort-package-json's order #31

JoshuaKGoldberg opened this issue Sep 13, 2023 · 5 comments · Fixed by #55
Assignees
Labels
status: accepting prs Please, send a pull request to resolve this! type: feature New enhancement or request

Comments

@JoshuaKGoldberg
Copy link
Owner

JoshuaKGoldberg commented Sep 13, 2023

Right now, the package-json/order-properties rule uses its own "standard order":

I haven't seen this order in recent history. But, I have seen https://github.com/keithamus/sort-package-json be used in recent ecosystem projects such as https://github.com/matzkoh/prettier-plugin-packagejson. Perhaps order-properties could take in an option?

cc @zetlen - is there a good reference for the current order? I'm wondering what to name if it there is an option added...

cc @kellyselden from https://github.com/kellyselden/eslint-plugin-json-files/blob/3a24e6f36b7828210a56070af4487cb5831f7f24/docs/rules/sort-package-json.md - I do find it interesting that both of these ESLint plugins have package.json sorting implemented...

@JoshuaKGoldberg JoshuaKGoldberg added type: feature New enhancement or request status: in discussion Not yet ready for implementation or a pull request labels Sep 13, 2023
@Zamiell
Copy link
Contributor

Zamiell commented Sep 13, 2023

I think that most of the ecosystem uses the sort-package-json order.
Most notably, as Josh said above, prettier-plugin-packagejson uses this order too.
Thus, I would vote that this plugin matches the ordering of sort-package-json by default.

i.e. lets avoid this:

alt

@zetlen
Copy link
Collaborator

zetlen commented Sep 13, 2023

I think I took the ordering from the order in which the documentation page describes them. It doesn't say anywhere that this is a required order. If there's another, more widespread convention, use that.

@JoshuaKGoldberg JoshuaKGoldberg added status: accepting prs Please, send a pull request to resolve this! and removed status: in discussion Not yet ready for implementation or a pull request labels Sep 13, 2023
@JoshuaKGoldberg
Copy link
Owner Author

Awesome! Let's go with, say, order: "legacy" | "sort-package-json"``? With "legacy"as the default now, noted to switch to"sort-package-json"` in a potential future version?

Marking as accepting PRs. If nobody's sent a PR by the time I've overhauled this repo (#33 through #37) I'll go ahead and send one myself.

Note that adding sort-package-json as a dependency might clobber package-lock.json until #33 is in. I've found that using npx npm@6 to run an older version of npm is a workaround.

@kendallgassner
Copy link
Collaborator

@JoshuaKGoldberg can you assign this ticket to me. I am going to take a look today.

JoshuaKGoldberg added a commit that referenced this issue Oct 17, 2023
closes:
#31


### What

- Replaced the `order-properties` sorting algorithm to use
[sort-package-json](https://github.com/keithamus/sort-package-json).
- Changed the type of `options` from an `Array<string>` to 
```tsx
{
    order?: "legacy" | "sort-package-json" | Array<string>
}
```
- defaulted the sort order to `legacy`
- Add additional testing to test options: "legacy" | "sort-package-json"
.


### Callouts

#### Option: `order: Array<string>`

Previously, any property that was not provided in the `order:
Array<string>` collection was ignored by the sort algorithm. Now that we
are using `sort-package-json`, [a property that is not provided in the
`order: Array<string>` will be sorted by the `defaultSortOrder` provided
by the
library.](https://github.com/keithamus/sort-package-json#optionssortorder)

#### Sort-package-json version
the latest version of sort-package-json is [an esm
module](https://github.com/keithamus/sort-package-json/blob/main/package.json#L20)
so I opted to use an earlier version to avoid heavy refactorign. This
package can be bumped when #33 is merged.

---------

Co-authored-by: Josh Goldberg <[email protected]>
@JoshuaKGoldberg
Copy link
Owner Author

Published as [email protected].

Intentionally a different minor version than the previous 0.1.x line. I'll publish #40 and #58 in a new 0.3 line so folks can stick with this feature in case ESLint 8 support breaks them.

Thanks again @kendallgassner!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: accepting prs Please, send a pull request to resolve this! type: feature New enhancement or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants