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.

Sign up for GitHub

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

Jump to bottom

Use same sorting logic for sorting dependencies as npm v7 #234

Open
G-Rath opened this issue Oct 26, 2021 · 10 comments · May be fixed by #304
Open

Use same sorting logic for sorting dependencies as npm v7 #234

G-Rath opened this issue Oct 26, 2021 · 10 comments · May be fixed by #304

Comments

@G-Rath
Copy link
Contributor

G-Rath commented Oct 26, 2021

NPM v7 changed the way dependencies are sorted very slightly which means this:

{
  "name": "spj-te",
  "dependencies": {
    "sort-package-json": "^1.52.0"
  },
  "devDependencies": {
    "@types/koa-bodyparser": "^4.3.3",
    "@types/koa__router": "^8.0.8"
  }
}

becomes this:

{
  "name": "spj-te",
  "dependencies": {
    "sort-package-json": "^1.52.0"
  },
  "devDependencies": {
    "@types/koa__router": "^8.0.8",
    "@types/koa-bodyparser": "^4.3.3"
  }
}

The specific change they made was to use localeCompare, which technically the better way for sorting strings than just calling sort but means npm v7 & above sort differently compared to both sort-package-json and the other two main package managers.

I opened an issue for discussion about this but it was closed with a comment about the change being intentional: npm/cli#3935

I was thinking that sort-package-json could attempt to figure out if it should use localeCompare by trying to read package-lock.json and checking if the lockfileVersion is greater than 1, otherwise falling back to its current behaviour if its 1 or the lock cannot be read.

I'm happy to implement this change.
@keithamus
Copy link
Owner

I'd be quite happy to just unilaterally change to using localeCompare, if that seems like a good change to you?

@G-Rath
Copy link
Contributor Author

G-Rath commented Oct 26, 2021

@keithamus yup I think in the long-run it makes sense, so if you're happy to start using it now I'll make a PR.

@G-Rath
Copy link
Contributor Author

G-Rath commented Oct 26, 2021

hmm I've applied this but the output is still not what npm v7 is giving, so will need to look into this further.

@Nokel81
Copy link

Nokel81 commented Dec 6, 2023

Any update on this? Would be nice to have the dependencies sorted the same order as npm has them

@G-Rath
Copy link
Contributor Author

G-Rath commented Dec 6, 2023

@Nokel81 I've been really on the fence about this because NPM v7+ actually sorts slightly different to all other package managers and they're not going to change that because apparently it'll create too much churn for NPM v7+ users...

In my experience I've only ever had this once on a project, with the dependencies mentioned above since it only impacts the sort order of - vs _, which is very rare to have packages matching except for those characters.

@Nokel81
Copy link

Nokel81 commented Dec 6, 2023

I noticed it specifically because of the dash and underscore. Due to ansi_up and ansi-exp.

I definitely think that fit projects using npm should be able to specify this sort order

@keithamus
Copy link
Owner

We could also detect what npm version is being used and do the right thing.

I'm happy to look over any PR that anyone wants to write on this 😉

@G-Rath
Copy link
Contributor Author

G-Rath commented Dec 6, 2023

Actually with packageManager being in package.json, that should be very easy to do 🤔

@Nokel81
Copy link

Nokel81 commented Dec 6, 2023

Or even with .engines.npm being set

@Nokel81
Copy link

Nokel81 commented Dec 6, 2023

Okay I can try

@Nokel81 Nokel81 linked a pull request Dec 6, 2023 that will close this issue
Open
@Zamiell Zamiell mentioned this issue Sep 21, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

feat: Introduce option for sorting deps in the same ordering as npm Nokel81/sort-package-json
3 participants
@keithamus @G-Rath @Nokel81