[BUG] dependency sort order has changed slightly

[G-Rath]

Is there an existing issue for this?

• I have searched the existing issues

Current Behavior

Given the following package.json:

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

Running:

npm install sort-package-json

Gives:

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

Expected Behavior

I'd expect the package.json not to have changed.

This expected sort ordering is what appears to currently be used by:

• npm v6
• yarn v1 & v2
• pnpm v6
• sort-package-json

Obviously this is a cosmetic bug and (afaik) there isn't a defined standard on the order that should be used in the ecosystem, but given it looks like just npm v7/8 is the odd duck I figured I'd open an issue to see if folks would be open to a PR, as it makes for noisy diffs to have packages swap back and forward 😅

I'm happy to help do the work - I suspect based on #1766 it could be a matter of adjusting the code added in this commit having looked a bit more into this, it looks like this commits function has gone away so would appreciate being pointed in the right direction

Steps To Reproduce

See Current Behaviour

Environment

• OS: Ubuntu 18.04
• Node: 16.0.0
• npm: v8 (latest, via npx), and v7 (latest, via npx)

[G-Rath]

Having looked a bit further into this, I'm wondering if this should be reported against npm/arborist instead as it seems like that is what's responsible for the ordering.

[G-Rath]

Nope actually looks like this belongs npm/package-json (which is where that commits code has moved to), and that the issue is the use of localeCompare :/

[fritzy]

This change was intentional to make sorting idempotent.

[G-Rath]

@fritzy fwiw it was pointed out to me that sort by itself actually already is idempotent, so not sure the use of localeCompare actually adds much vs breaking consistency.

[G-Rath]

@fritzy would it be possible to re-open this issue for further discussion? While I know it seems very inconsequential, my role has me maintaining a wide range of projects which use different package managers and tools so this little bit of inconsistency adds unnecessary overhead to the mental thinking I have to do by creating nosier diffs.

Ultimately what's on my mind with this is that .sort() is already idempotent, and in fact localeCompare could have more overhead (It has already resulted in one extra dependency being pulled in, which'd otherwise not be needed) - so I don't think there's any actual advantage to npm using localeCompare like this, since the array contents are strings.

[ljharb]

.sort is stable now, but i'm not sure which node version that landed in, and it might still not be stable in every version npm supports.

[G-Rath]

@ljharb v12 according to MDN

Which is the minimal version supported by npm v8 now

[ljharb]

Fair point, it's in v12.0.0 and npm requires even higher than that.

However, the default sort comparator still might not have the desired behavior.