fix: use Intl.Collator for string sorting when available

Credit: @killagu
Close: #317

PR-URL: #324
Credit: @isaacs
Close: #324
Reviewed-by: @isaacs

Author: isaacs

lib/add-rm-pkg-deps.js

@@ -1,5 +1,7 @@
 // add and remove dependency specs to/from pkg manifest
 
+const localeCompare = require('@isaacs/string-locale-compare')('en')
+
 const add = ({pkg, add, saveBundle, saveType, log}) => {
   for (const spec of add) {
     addSingle({pkg, spec, saveBundle, saveType, log})
@@ -79,7 +81,7 @@ const addSingle = ({pkg, spec, saveBundle, saveType, log}) => {
     // keep it sorted, keep it unique
     const bd = new Set(pkg.bundleDependencies || [])
     bd.add(spec.name)
-    pkg.bundleDependencies = [...bd].sort((a, b) => a.localeCompare(b, 'en'))
+    pkg.bundleDependencies = [...bd].sort(localeCompare)
   }
 }
 

lib/arborist/build-ideal-tree.js

@@ -1,4 +1,5 @@
 // mixin implementing the buildIdealTree method
+const localeCompare = require('@isaacs/string-locale-compare')('en')
 const rpj = require('read-package-json-fast')
 const npa = require('npm-package-arg')
 const pacote = require('pacote')
@@ -771,7 +772,7 @@ This is a one-time fix-up, please be patient...
     // sort physically shallower deps up to the front of the queue,
     // because they'll affect things deeper in, then alphabetical
     this[_depsQueue].sort((a, b) =>
-      (a.depth - b.depth) || a.path.localeCompare(b.path, 'en'))
+      (a.depth - b.depth) || localeCompare(a.path, b.path))
 
     const node = this[_depsQueue].shift()
     const bd = node.package.bundleDependencies
@@ -916,7 +917,7 @@ This is a one-time fix-up, please be patient...
     }
 
     const placeDeps = tasks
-      .sort((a, b) => a.edge.name.localeCompare(b.edge.name, 'en'))
+      .sort((a, b) => localeCompare(a.edge.name, b.edge.name))
       .map(({ edge, dep }) => new PlaceDep({
         edge,
         dep,
@@ -1247,7 +1248,7 @@ This is a one-time fix-up, please be patient...
       // we typically only install non-optional peers, but we have to
       // factor them into the peerSet so that we can avoid conflicts
       .filter(e => e.peer && !(e.valid && e.to))
-      .sort(({name: a}, {name: b}) => a.localeCompare(b, 'en'))
+      .sort(({name: a}, {name: b}) => localeCompare(a, b))
 
     for (const edge of peerEdges) {
       // already placed this one, and we're happy with it.

lib/arborist/load-virtual.js

@@ -1,4 +1,5 @@
 // mixin providing the loadVirtual method
+const localeCompare = require('@isaacs/string-locale-compare')('en')
 
 const {resolve} = require('path')
 
@@ -167,12 +168,12 @@ module.exports = cls => class VirtualLoader extends cls {
       ...depsToEdges('peerOptional', peerOptional),
       ...lockWS,
     ].sort(([atype, aname], [btype, bname]) =>
-      atype.localeCompare(btype, 'en') || aname.localeCompare(bname, 'en'))
+      localeCompare(atype, btype) || localeCompare(aname, bname))
 
     const rootEdges = [...root.edgesOut.values()]
       .map(e => [e.type, e.name, e.spec])
       .sort(([atype, aname], [btype, bname]) =>
-        atype.localeCompare(btype, 'en') || aname.localeCompare(bname, 'en'))
+        localeCompare(atype, btype) || localeCompare(aname, bname))
 
     if (rootEdges.length !== lockEdges.length) {
       // something added or removed

lib/arborist/rebuild.js

@@ -1,6 +1,7 @@
 // Arborist.rebuild({path = this.path}) will do all the binlinks and
 // bundle building needed.  Called by reify, and by `npm rebuild`.
 
+const localeCompare = require('@isaacs/string-locale-compare')('en')
 const {depth: dfwalk} = require('treeverse')
 const promiseAllRejectLate = require('promise-all-reject-late')
 const rpj = require('read-package-json-fast')
@@ -14,7 +15,8 @@ const {
 } = require('@npmcli/node-gyp')
 
 const boolEnv = b => b ? '1' : ''
-const sortNodes = (a, b) => (a.depth - b.depth) || a.path.localeCompare(b.path, 'en')
+const sortNodes = (a, b) =>
+  (a.depth - b.depth) || localeCompare(a.path, b.path)
 
 const _workspaces = Symbol.for('workspaces')
 const _build = Symbol('build')

lib/audit-report.js

@@ -1,6 +1,7 @@
 // an object representing the set of vulnerabilities in a tree
 /* eslint camelcase: "off" */
 
+const localeCompare = require('@isaacs/string-locale-compare')('en')
 const npa = require('npm-package-arg')
 const pickManifest = require('npm-pick-manifest')
 
@@ -79,7 +80,7 @@ class AuditReport extends Map {
     }
 
     obj.vulnerabilities = vulnerabilities
-      .sort(([a], [b]) => a.localeCompare(b, 'en'))
+      .sort(([a], [b]) => localeCompare(a, b))
       .reduce((set, [name, vuln]) => {
         set[name] = vuln
         return set

lib/can-place-dep.js

@@ -35,6 +35,7 @@
 // then we will return REPLACE rather than CONFLICT, and Arborist will queue
 // the replaced node for resolution elsewhere.
 
+const localeCompare = require('@isaacs/string-locale-compare')('en')
 const semver = require('semver')
 const debug = require('./debug.js')
 const peerEntrySets = require('./peer-entry-sets.js')
@@ -79,7 +80,7 @@ class CanPlaceDep {
       this._treeSnapshot = JSON.stringify([...target.root.inventory.entries()]
         .map(([loc, {packageName, version, resolved}]) => {
           return [loc, packageName, version, resolved]
-        }).sort(([a], [b]) => a.localeCompare(b, 'en')))
+        }).sort(([a], [b]) => localeCompare(a, b)))
     })
 
     // the result of whether we can place it or not
@@ -119,7 +120,7 @@ class CanPlaceDep {
       const treeSnapshot = JSON.stringify([...target.root.inventory.entries()]
         .map(([loc, {packageName, version, resolved}]) => {
           return [loc, packageName, version, resolved]
-        }).sort(([a], [b]) => a.localeCompare(b, 'en')))
+        }).sort(([a], [b]) => localeCompare(a, b)))
       /* istanbul ignore if */
       if (this._treeSnapshot !== treeSnapshot) {
         throw Object.assign(new Error('tree changed in CanPlaceDep'), {

lib/place-dep.js

@@ -7,6 +7,7 @@
 // and saves a set of what was placed and what needs re-evaluation as
 // a result.
 
+const localeCompare = require('@isaacs/string-locale-compare')('en')
 const log = require('proc-log')
 const deepestNestingTarget = require('./deepest-nesting-target.js')
 const CanPlaceDep = require('./can-place-dep.js')
@@ -473,7 +474,7 @@ class PlaceDep {
       // sort these so that they're deterministically ordered
       // otherwise, resulting tree shape is dependent on the order
       // in which they happened to be resolved.
-      const nodeSort = (a, b) => a.location.localeCompare(b.location, 'en')
+      const nodeSort = (a, b) => localeCompare(a.location, b.location)
 
       const children = [...node.children.values()].sort(nodeSort)
       for (const child of children) {

lib/printable.js

@@ -1,6 +1,7 @@
 // helper function to output a clearer visualization
 // of the current node and its descendents
 
+const localeCompare = require('@isaacs/string-locale-compare')('en')
 const util = require('util')
 const relpath = require('./relpath.js')
 
@@ -67,14 +68,14 @@ class ArboristNode {
     // edgesOut sorted by name
     if (tree.edgesOut.size) {
       this.edgesOut = new Map([...tree.edgesOut.entries()]
-        .sort(([a], [b]) => a.localeCompare(b, 'en'))
+        .sort(([a], [b]) => localeCompare(a, b))
         .map(([name, edge]) => [name, new EdgeOut(edge)]))
     }
 
     // edgesIn sorted by location
     if (tree.edgesIn.size) {
       this.edgesIn = new Set([...tree.edgesIn]
-        .sort((a, b) => a.from.location.localeCompare(b.from.location, 'en'))
+        .sort((a, b) => localeCompare(a.from.location, b.from.location))
         .map(edge => new EdgeIn(edge)))
     }
 
@@ -86,14 +87,14 @@ class ArboristNode {
     // fsChildren sorted by path
     if (tree.fsChildren.size) {
       this.fsChildren = new Set([...tree.fsChildren]
-        .sort(({path: a}, {path: b}) => a.localeCompare(b, 'en'))
+        .sort(({path: a}, {path: b}) => localeCompare(a, b))
         .map(tree => printableTree(tree, path)))
     }
 
     // children sorted by name
     if (tree.children.size) {
       this.children = new Map([...tree.children.entries()]
-        .sort(([a], [b]) => a.localeCompare(b, 'en'))
+        .sort(([a], [b]) => localeCompare(a, b))
         .map(([name, tree]) => [name, printableTree(tree, path)]))
     }
   }

lib/shrinkwrap.js

@@ -9,6 +9,7 @@
 // We cannot bump to v3 until npm v6 is out of common usage, and
 // definitely not before npm v8.
 
+const localeCompare = require('@isaacs/string-locale-compare')('en')
 const lockfileVersion = 2
 
 // for comparing nodes to yarn.lock entries
@@ -911,7 +912,7 @@ class Shrinkwrap {
       /* istanbul ignore next - sort calling order is indeterminate */
       return aloc.length > bloc.length ? 1
         : bloc.length > aloc.length ? -1
-        : aloc[aloc.length - 1].localeCompare(bloc[bloc.length - 1], 'en')
+        : localeCompare(aloc[aloc.length - 1], bloc[bloc.length - 1])
     })[0]
 
     const res = consistentResolve(node.resolved, this.path, this.path, true)

lib/vuln.js

@@ -14,6 +14,7 @@
 const {satisfies, simplifyRange} = require('semver')
 const semverOpt = { loose: true, includePrerelease: true }
 
+const localeCompare = require('@isaacs/string-locale-compare')('en')
 const npa = require('npm-package-arg')
 const _range = Symbol('_range')
 const _simpleRange = Symbol('_simpleRange')
@@ -112,12 +113,10 @@ class Vuln {
         vulnerableVersions: undefined,
         id: undefined,
       }).sort((a, b) =>
-        String(a.source || a).localeCompare(String(b.source || b, 'en'))),
-      effects: [...this.effects].map(v => v.name)
-        .sort(/* istanbul ignore next */(a, b) => a.localeCompare(b, 'en')),
+        localeCompare(String(a.source || a), String(b.source || b))),
+      effects: [...this.effects].map(v => v.name).sort(localeCompare),
       range: this.simpleRange,
-      nodes: [...this.nodes].map(n => n.location)
-        .sort(/* istanbul ignore next */(a, b) => a.localeCompare(b, 'en')),
+      nodes: [...this.nodes].map(n => n.location).sort(localeCompare),
       fixAvailable: this[_fixAvailable],
     }
   }