feat: sort dependencies with npm algorithm, sort npm Overrides key

[karlhorky]

Closes #234
Closes #355
Closes #267

As per @keithamus' comment in #355, sorting via the npm CLI's algorithm would resolve #234 and #355

• Add the 'overrides' key for npm Overrides
• Sort overrides, dependencies, devDependencies, peerDependencies, optionalDependencies with npm algorithm

Not sure if this is the preferred way of adding this change - happy to continue this to make this ready to accept

[karlhorky]

also introduces sorting for the 'overrides' key for npm Overrides

[G-Rath]

This looks good to me, though you should update/add some tests ideally and also flagging I personally would prefer if this tried to check what package manager was being used since I think that'll be very cheap and the dependency objects are the only ones package managers actually sort themselves.

I'm happy to add that myself as a follow-up but want to mention it here all the same to confirm people are agreeable to that

[karlhorky]

you should update/add some tests ideally

Good idea, I'll take a look at this.

I personally would prefer if this tried to check what package manager was being used

Hmm... although I can follow the idea behind it, I think I would have reservations with that:

1. The localeCompare sorting from npm CLI's algorithm makes more sense than the current sorting from sort-package-json - check my issue Sorting of package names with capital letters doesn't match npm #355 for how sort-package-json currently sorts package names starting with capital letters before package names starting with lowercase letters, which seems unusual to me on a human level
2. That sorting change was made in npm CLI at least 4 years ago (maybe very old npm versions don't need to be supported by sort-package-json?)

[G-Rath]

I think I would have reservations with that:

NPM isn't the only package manager though, and it's the only one that does this method of sorting - that frustrates me to no end because it feels like a completely unneeded difference in our ecosystem, but when I tried talking about that with NPM they said they were not going to change back because (ironically) they felt it would be too much churn for NPM v7+ users, and I just don't see it being something that every really changes.

I also would love to use the same package manager everywhere, but that too is something I don't think will ever be "solved" as the big four have their own merits and fans meaning you probably shouldn't rule out using a particular one indefinitely - I frequently have to jump between the original three due to my open source work.

The reason why I care for it respecting the package manager is that this a tool that often only needs to get run on occasion as the majority of content in package.json are dependencies which as mentioned package managers handle sorting, so after a first-run there's not a lot of value in running this in CI every single time, so we just do so manually as and when needed - and most of the time that's fine as you just run the tool and commit the result, but every so often you get a codebase with a dependency whose sort order will be different and suddenly the diff has this weird package swapping and you've got to explain that to the junior dev and so on.

Hence my desire to try and be what I consider a little more friendly by doing a couple of cheap attempts to guess what package manager is being used and thus what sort order, such as fs.stat('package-lock.json') or maybe looking for engines.npm.

(fwiw if you do ever want to talk to any of the other package managers about changing their order, feel free to tag me for my vote - I don't actually think they'd be opposed to changing it just requires someone being able to put the time in to get it across the line since it'll be hard to make a high priority)

[karlhorky]

Hm, ok understood.

What do you think about a different way: an option disableDepSorting (default false) that would leave dependencies, devDependencies, etc unsorted for those who want that? That could be used by users of those other package managers but also by other users who don't want dependencies to be sorted... 🤔

I guess I would prefer an option for disabling the sorting for dependencies altogether over maintaining the current (surprising) sorting behavior anywhere.

[karlhorky]

Added some simple tests in 3df9260 - added the objects inline because I didn't clone the repo yet, and they're easier to add in the GitHub interface than the Ava snapshots.

If the Ava snapshots need to be added, will do that too.

[G-Rath]

The CLI purposely does not support configuration so that's a no-go

[karlhorky]

The CLI purposely does not support configuration so that's a no-go

I see that now, in the no-configuration section in the readme:

The lack of configuration here is a feature, not a bug. The intent of this tool is that a user can open a package json and always expect to see keys in a particular order.

The structure of the package.json should always be predictable & deterministic from project to project.

However, reading this I also think that introducing any branching in the dep-sorting behavior based on package manager would also be against this same philosophy (because it would be "keys in a different order").

I have no horse in the race here - so I'm happy to go with whatever the maintainer team here decides. But I would suggest getting rid of the current dependencies sorting, because the uppercase-before-lowercase sorting doesn't make sense the way it is currently.

@keithamus any opinion here? Options I would suggest:

1. Keep the tool consistently sorting regardless of package manager?
2. Or introduce some "magic" detection for non-npm and disable dependency sorting for those?

[G-Rath]

However, reading this I also think that introducing any branching in the dep-sorting behavior based on package manager would also be against this same philosophy (because it would be "keys in a different order").

No because the sort would still be predictable and deterministic, like how the sorting of scripts is different depending on if dependencies like npm-run-all are present and if its --parallel flag is being used in scripts

[keithamus]

Great work! Let’s merge this as is, and we can iterate further if people consider this change too problematic.

I’d like to avoid too many heuristics as with too many in place it can lead to a “sense” of indeterminism.

[github-actions]

🎉 This PR is included in version 3.2.0 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀