fix(packages): locale-agnostic string sorting

This adds the 'en' locale to all instances of String.localeCompare
within the CLI codebase.

Tests added for the cases where we're sorting arbitrary user-generated
data.  The tests rely on the fact that 'ch' sorts after 'd' in the
`'sk'` locale, but ahead of `'d'` in the `'en'` locale.  To ensure that
this is the default behavior if no locale is specified, `LC_ALL=sk` is
set in the test environment.

Other instances of `localeCompare` sort data that the cli controls, so
no tests were added.

Re: #2829

PR-URL: #3203
Credit: @isaacs
Close: #3203
Reviewed-by: @ruyadorno

Author: isaacs

lib/config.js

@@ -199,7 +199,7 @@ class Config extends BaseCommand {
 ; Configs like \`//<hostname>/:_authToken\` are auth that is restricted
 ; to the registry host specified.
 
-${data.split('\n').sort((a, b) => a.localeCompare(b)).join('\n').trim()}
+${data.split('\n').sort((a, b) => a.localeCompare(b, 'en')).join('\n').trim()}
 
 ;;;;
 ; all available options shown below with default values
@@ -227,7 +227,7 @@ ${defData}
       if (where === 'default' && !long)
         continue
 
-      const keys = Object.keys(data).sort((a, b) => a.localeCompare(b))
+      const keys = Object.keys(data).sort((a, b) => a.localeCompare(b, 'en'))
       if (!keys.length)
         continue
 

lib/help.js

@@ -77,7 +77,7 @@ class Help extends BaseCommand {
       if (aManNumber !== bManNumber)
         return aManNumber - bManNumber
 
-      return a.localeCompare(b)
+      return a.localeCompare(b, 'en')
     })
     const man = mans[0]
 

lib/ls.js

@@ -443,7 +443,7 @@ const augmentNodesWithMetadata = ({
 }
 
 const sortAlphabetically = (a, b) =>
-  a.pkgid.localeCompare(b.pkgid)
+  a.pkgid.localeCompare(b.pkgid, 'en')
 
 const humanOutput = ({ color, result, seenItems, unicode }) => {
   // we need to traverse the entire tree in order to determine which items

lib/outdated.js

@@ -68,7 +68,7 @@ class Outdated extends BaseCommand {
     }))
 
     // sorts list alphabetically
-    const outdated = this.list.sort((a, b) => a.name.localeCompare(b.name))
+    const outdated = this.list.sort((a, b) => a.name.localeCompare(b.name, 'en'))
 
     // return if no outdated packages
     if (outdated.length === 0 && !this.npm.config.get('json'))

lib/utils/completion/installed-deep.js

@@ -16,7 +16,7 @@ const installedDeep = async (npm) => {
       })
       .filter(i => (i.depth - 1) <= depth)
       .sort((a, b) => a.depth - b.depth)
-      .sort((a, b) => a.depth === b.depth ? a.name.localeCompare(b.name) : 0)
+      .sort((a, b) => a.depth === b.depth ? a.name.localeCompare(b.name, 'en') : 0)
 
   const res = new Set()
   const gArb = new Arborist({ global: true, path: resolve(npm.globalDir, '..') })

lib/utils/config/describe-all.js

@@ -7,7 +7,7 @@ const describeAll = () => {
   const sort = ([keya, {deprecated: depa}], [keyb, {deprecated: depb}]) => {
     return depa && !depb ? 1
       : !depa && depb ? -1
-      : keya.localeCompare(keyb)
+      : keya.localeCompare(keyb, 'en')
   }
   return Object.entries(definitions).sort(sort)
     .map(([key, def]) => def.describe())

lib/utils/npm-usage.js

@@ -62,7 +62,7 @@ const usages = (npm) => {
     maxLen = Math.max(maxLen, c.length)
     return set
   }, [])
-    .sort((a, b) => a[0].localeCompare(b[0]))
+    .sort((a, b) => a[0].localeCompare(b[0], 'en'))
     .map(([c, usage]) => `\n    ${c}${' '.repeat(maxLen - c.length + 1)}${
       (usage.split('\n').join('\n' + ' '.repeat(maxLen + 5)))}`)
     .join('\n')

lib/utils/tar.js

@@ -76,15 +76,15 @@ const getContents = async (manifest, tarball) => {
   })
 
   const comparator = (a, b) => {
-    return a.path.localeCompare(b.path, undefined, {
+    return a.path.localeCompare(b.path, 'en', {
       sensitivity: 'case',
       numeric: true,
     })
   }
 
   const isUpper = (str) => {
     const ch = str.charAt(0)
-    return ch >= 'A' && ch <= 'Z'
+    return ch === ch.toUpperCase()
   }
 
   const uppers = files.filter(file => isUpper(file.path))

package.json

@@ -216,6 +216,9 @@
     "Remove the 'files' below once we're done porting old tests over"
   ],
   "tap": {
+    "test-env": [
+      "LC_ALL=sk"
+    ],
     "color": 1,
     "files": "test/{lib,bin}",
     "coverage-map": "test/coverage-map.js",

scripts/bundle-and-gitignore-deps.js

@@ -18,9 +18,9 @@ arb.loadVirtual().then(tree => {
       bundle.push(node.name)
     }
   }
-  pkg.bundleDependencies = bundle.sort((a, b) => a.localeCompare(b))
+  pkg.bundleDependencies = bundle.sort((a, b) => a.localeCompare(b, 'en'))
 
-  const ignores = shouldIgnore.sort((a, b) => a.localeCompare(b))
+  const ignores = shouldIgnore.sort((a, b) => a.localeCompare(b, 'en'))
     .map(i => `/${i}`)
     .join('\n')
   const ignoreData = `# Automatically generated to ignore dev deps

scripts/config-doc.js

@@ -35,8 +35,8 @@ const addShorthands = doc => {
   const body = Object.entries(shorthands)
     .sort(([shorta, expansiona], [shortb, expansionb]) => {
       // sort by what they're short FOR
-      return expansiona.join(' ').localeCompare(expansionb.join(' ')) ||
-        shorta.localeCompare(shortb)
+      return expansiona.join(' ').localeCompare(expansionb.join(' '), 'en') ||
+        shorta.localeCompare(shortb, 'en')
     })
     .map(([short, expansion]) => {
     const dash = short.length === 1 ? '-' : '--'

tap-snapshots/test/lib/config.js.test.cjs

@@ -88,6 +88,9 @@ exports[`test/lib/config.js TAP config edit > should write config file 2`] = `
 exports[`test/lib/config.js TAP config get no args > should list configs on config get no args 1`] = `
 ; "cli" config from command line options
 
+cat = true 
+chai = true 
+dog = true 
 editor = "vi" 
 global = false 
 json = false 
@@ -109,6 +112,9 @@ init.version = "1.0.0"
 
 ; "cli" config from command line options
 
+cat = true 
+chai = true 
+dog = true 
 editor = "vi" 
 global = false 
 json = false 
@@ -118,6 +124,9 @@ long = true
 exports[`test/lib/config.js TAP config list > should list configs 1`] = `
 ; "cli" config from command line options
 
+cat = true 
+chai = true 
+dog = true 
 editor = "vi" 
 global = false 
 json = false 
@@ -132,6 +141,9 @@ long = false
 exports[`test/lib/config.js TAP config list overrides > should list overridden configs 1`] = `
 ; "cli" config from command line options
 
+cat = true 
+chai = true 
+dog = true 
 editor = "vi" 
 global = false 
 init.author.name = "Bar"