From dbb90f7997900b8ae6026dddaa718efe9a1db2f4 Mon Sep 17 00:00:00 2001 From: isaacs Date: Tue, 28 Sep 2021 11:18:01 -0700 Subject: [PATCH] fix: use Intl.Collator for string sorting when available The npm/cli form of https://github.com/npm/arborist/pull/324 Required adding options support to package used for this. PR-URL: https://github.com/npm/cli/pull/3809 Credit: @isaacs Close: #3809 Reviewed-by: @wraithgar --- lib/cache.js | 5 +-- lib/config.js | 5 +-- lib/help.js | 3 +- lib/ls.js | 4 +-- lib/outdated.js | 3 +- lib/utils/completion/installed-deep.js | 4 +-- lib/utils/config/describe-all.js | 3 +- lib/utils/npm-usage.js | 3 +- lib/utils/tar.js | 11 +++--- .../@isaacs/string-locale-compare/index.js | 36 ++++++++++++++----- .../string-locale-compare/package.json | 2 +- package-lock.json | 13 +++---- package.json | 1 + 13 files changed, 60 insertions(+), 33 deletions(-) diff --git a/lib/cache.js b/lib/cache.js index aed2cce31e63f..4a5665111949f 100644 --- a/lib/cache.js +++ b/lib/cache.js @@ -8,6 +8,7 @@ const semver = require('semver') const BaseCommand = require('./base-command.js') const npa = require('npm-package-arg') const jsonParse = require('json-parse-even-better-errors') +const localeCompare = require('@isaacs/string-locale-compare')('en') const searchCachePackage = async (path, spec, cacheKeys) => { const parsed = npa(spec) @@ -212,10 +213,10 @@ class Cache extends BaseCommand { for (const key of keySet) results.add(key) } - [...results].sort((a, b) => a.localeCompare(b, 'en')).forEach(key => this.npm.output(key)) + [...results].sort(localeCompare).forEach(key => this.npm.output(key)) return } - cacheKeys.sort((a, b) => a.localeCompare(b, 'en')).forEach(key => this.npm.output(key)) + cacheKeys.sort(localeCompare).forEach(key => this.npm.output(key)) } } diff --git a/lib/config.js b/lib/config.js index 2df7bf513437c..a1f706d930e6a 100644 --- a/lib/config.js +++ b/lib/config.js @@ -10,6 +10,7 @@ const writeFile = promisify(fs.writeFile) const { spawn } = require('child_process') const { EOL } = require('os') const ini = require('ini') +const localeCompare = require('@isaacs/string-locale-compare')('en') // take an array of `[key, value, k2=v2, k3, v3, ...]` and turn into // { key: value, k2: v2, k3: v3 } @@ -209,7 +210,7 @@ class Config extends BaseCommand { ; Configs like \`///:_authToken\` are auth that is restricted ; to the registry host specified. -${data.split('\n').sort((a, b) => a.localeCompare(b, 'en')).join('\n').trim()} +${data.split('\n').sort(localeCompare).join('\n').trim()} ;;;; ; all available options shown below with default values @@ -238,7 +239,7 @@ ${defData} if (where === 'default' && !long) continue - const keys = Object.keys(data).sort((a, b) => a.localeCompare(b, 'en')) + const keys = Object.keys(data).sort(localeCompare) if (!keys.length) continue diff --git a/lib/help.js b/lib/help.js index 8e4ff67bc284c..9a6f950e05953 100644 --- a/lib/help.js +++ b/lib/help.js @@ -3,6 +3,7 @@ const path = require('path') const openUrl = require('./utils/open-url.js') const { promisify } = require('util') const glob = promisify(require('glob')) +const localeCompare = require('@isaacs/string-locale-compare')('en') const BaseCommand = require('./base-command.js') @@ -82,7 +83,7 @@ class Help extends BaseCommand { if (aManNumber !== bManNumber) return aManNumber - bManNumber - return a.localeCompare(b, 'en') + return localeCompare(a, b) }) const man = mans[0] diff --git a/lib/ls.js b/lib/ls.js index 7e41892c53442..495b6348a0360 100644 --- a/lib/ls.js +++ b/lib/ls.js @@ -22,6 +22,7 @@ const _problems = Symbol('problems') const _required = Symbol('required') const _type = Symbol('type') const ArboristWorkspaceCmd = require('./workspaces/arborist-cmd.js') +const localeCompare = require('@isaacs/string-locale-compare')('en') class LS extends ArboristWorkspaceCmd { /* istanbul ignore next - see test/lib/load-all-commands.js */ @@ -503,8 +504,7 @@ const augmentNodesWithMetadata = ({ return node } -const sortAlphabetically = (a, b) => - a.pkgid.localeCompare(b.pkgid, 'en') +const sortAlphabetically = ({ pkgid: a }, { pkgid: b }) => localeCompare(a, b) const humanOutput = ({ color, result, seenItems, unicode }) => { // we need to traverse the entire tree in order to determine which items diff --git a/lib/outdated.js b/lib/outdated.js index a5be13cdaf108..b3b630421c488 100644 --- a/lib/outdated.js +++ b/lib/outdated.js @@ -6,6 +6,7 @@ const color = require('chalk') const styles = require('ansistyles') const npa = require('npm-package-arg') const pickManifest = require('npm-pick-manifest') +const localeCompare = require('@isaacs/string-locale-compare')('en') const Arborist = require('@npmcli/arborist') @@ -85,7 +86,7 @@ class Outdated extends ArboristWorkspaceCmd { })) // sorts list alphabetically - const outdated = this.list.sort((a, b) => a.name.localeCompare(b.name, 'en')) + const outdated = this.list.sort((a, b) => localeCompare(a.name, b.name)) if (outdated.length > 0) process.exitCode = 1 diff --git a/lib/utils/completion/installed-deep.js b/lib/utils/completion/installed-deep.js index 2430688612cd4..590955a1ee520 100644 --- a/lib/utils/completion/installed-deep.js +++ b/lib/utils/completion/installed-deep.js @@ -1,5 +1,6 @@ const { resolve } = require('path') const Arborist = require('@npmcli/arborist') +const localeCompare = require('@isaacs/string-locale-compare')('en') const installedDeep = async (npm) => { const { @@ -15,8 +16,7 @@ const installedDeep = async (npm) => { return i }) .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, 'en') : 0) + .sort((a, b) => (a.depth - b.depth) || localeCompare(a.name, b.name)) const res = new Set() const gArb = new Arborist({ global: true, path: resolve(npm.globalDir, '..') }) diff --git a/lib/utils/config/describe-all.js b/lib/utils/config/describe-all.js index c8a973cc0fd21..23a10ae97783c 100644 --- a/lib/utils/config/describe-all.js +++ b/lib/utils/config/describe-all.js @@ -1,4 +1,5 @@ const definitions = require('./definitions.js') +const localeCompare = require('@isaacs/string-locale-compare')('en') const describeAll = () => { // sort not-deprecated ones to the top /* istanbul ignore next - typically already sorted in the definitions file, @@ -7,7 +8,7 @@ const describeAll = () => { const sort = ([keya, {deprecated: depa}], [keyb, {deprecated: depb}]) => { return depa && !depb ? 1 : !depa && depb ? -1 - : keya.localeCompare(keyb, 'en') + : localeCompare(keya, keyb) } return Object.entries(definitions).sort(sort) .map(([key, def]) => def.describe()) diff --git a/lib/utils/npm-usage.js b/lib/utils/npm-usage.js index ddb0bab0bc9a2..f6785867c0ae5 100644 --- a/lib/utils/npm-usage.js +++ b/lib/utils/npm-usage.js @@ -1,5 +1,6 @@ const { dirname } = require('path') const { cmdList } = require('./cmd-list') +const localeCompare = require('@isaacs/string-locale-compare')('en') module.exports = (npm) => { const usesBrowser = npm.config.get('viewer') === 'browser' @@ -62,7 +63,7 @@ const usages = (npm) => { maxLen = Math.max(maxLen, c.length) return set }, []) - .sort((a, b) => a[0].localeCompare(b[0], 'en')) + .sort(([a], [b]) => localeCompare(a, b)) .map(([c, usage]) => `\n ${c}${' '.repeat(maxLen - c.length + 1)}${ (usage.split('\n').join('\n' + ' '.repeat(maxLen + 5)))}`) .join('\n') diff --git a/lib/utils/tar.js b/lib/utils/tar.js index c3071c1bd47a5..0ff822370d4da 100644 --- a/lib/utils/tar.js +++ b/lib/utils/tar.js @@ -3,6 +3,10 @@ const ssri = require('ssri') const npmlog = require('npmlog') const formatBytes = require('./format-bytes.js') const columnify = require('columnify') +const localeCompare = require('@isaacs/string-locale-compare')('en', { + sensitivity: 'case', + numeric: true, +}) const logTar = (tarball, opts = {}) => { const { unicode = false, log = npmlog } = opts @@ -75,12 +79,7 @@ const getContents = async (manifest, tarball) => { algorithms: ['sha1', 'sha512'], }) - const comparator = (a, b) => { - return a.path.localeCompare(b.path, 'en', { - sensitivity: 'case', - numeric: true, - }) - } + const comparator = ({ path: a }, { path: b }) => localeCompare(a, b) const isUpper = (str) => { const ch = str.charAt(0) diff --git a/node_modules/@isaacs/string-locale-compare/index.js b/node_modules/@isaacs/string-locale-compare/index.js index a6cec27efb8ec..0f68ab6774e1a 100644 --- a/node_modules/@isaacs/string-locale-compare/index.js +++ b/node_modules/@isaacs/string-locale-compare/index.js @@ -2,21 +2,41 @@ const hasIntl = typeof Intl === 'object' && !!Intl const Collator = hasIntl && Intl.Collator const cache = new Map() -const collatorCompare = locale => { - const collator = new Collator(locale) +const collatorCompare = (locale, opts) => { + const collator = new Collator(locale, opts) return (a, b) => collator.compare(a, b) } -const localeCompare = locale => (a, b) => a.localeCompare(b, locale) +const localeCompare = (locale, opts) => (a, b) => a.localeCompare(b, locale, opts) -module.exports = locale => { +const knownOptions = [ + 'sensitivity', + 'numeric', + 'ignorePunctuation', + 'caseFirst', +] + +const { hasOwnProperty } = Object.prototype + +module.exports = (locale, options = {}) => { if (!locale || typeof locale !== 'string') throw new TypeError('locale required') - if (cache.has(locale)) - return cache.get(locale) + const opts = knownOptions.reduce((opts, k) => { + if (hasOwnProperty.call(options, k)) { + opts[k] = options[k] + } + return opts + }, {}) + const key = `${locale}\n${JSON.stringify(opts)}` + + if (cache.has(key)) + return cache.get(key) + + const compare = hasIntl + ? collatorCompare(locale, opts) + : localeCompare(locale, opts) + cache.set(key, compare) - const compare = hasIntl ? collatorCompare(locale) : localeCompare(locale) - cache.set(locale, compare) return compare } diff --git a/node_modules/@isaacs/string-locale-compare/package.json b/node_modules/@isaacs/string-locale-compare/package.json index a322c1c92d3cc..58de848a00377 100644 --- a/node_modules/@isaacs/string-locale-compare/package.json +++ b/node_modules/@isaacs/string-locale-compare/package.json @@ -1,6 +1,6 @@ { "name": "@isaacs/string-locale-compare", - "version": "1.0.1", + "version": "1.1.0", "files": [ "index.js" ], diff --git a/package-lock.json b/package-lock.json index 246b517ab3764..e8cc960ecfec2 100644 --- a/package-lock.json +++ b/package-lock.json @@ -84,6 +84,7 @@ "packages/*" ], "dependencies": { + "@isaacs/string-locale-compare": "^1.1.0", "@npmcli/arborist": "^2.9.0", "@npmcli/ci-detect": "^1.2.0", "@npmcli/config": "^2.3.0", @@ -619,9 +620,9 @@ "dev": true }, "node_modules/@isaacs/string-locale-compare": { - "version": "1.0.1", - "resolved": "https://registry.npmjs.org/@isaacs/string-locale-compare/-/string-locale-compare-1.0.1.tgz", - "integrity": "sha512-AknEkBKSyAcIpl7SIUp12bs1rOmTDp9ojfDI9hvXl6qHqUCcaswkZOslbfdEbzI+8OPatiixY9AFKaUUpgGoBw==", + "version": "1.1.0", + "resolved": "https://registry.npmjs.org/@isaacs/string-locale-compare/-/string-locale-compare-1.1.0.tgz", + "integrity": "sha512-SQ7Kzhh9+D+ZW9MA0zkYv3VXhIDNx+LzM6EJ+/65I3QY+enU6Itte7E5XX7EWrqLW2FN4n06GWzBnPoC3th2aQ==", "inBundle": true }, "node_modules/@istanbuljs/load-nyc-config": { @@ -10908,9 +10909,9 @@ "dev": true }, "@isaacs/string-locale-compare": { - "version": "1.0.1", - "resolved": "https://registry.npmjs.org/@isaacs/string-locale-compare/-/string-locale-compare-1.0.1.tgz", - "integrity": "sha512-AknEkBKSyAcIpl7SIUp12bs1rOmTDp9ojfDI9hvXl6qHqUCcaswkZOslbfdEbzI+8OPatiixY9AFKaUUpgGoBw==" + "version": "1.1.0", + "resolved": "https://registry.npmjs.org/@isaacs/string-locale-compare/-/string-locale-compare-1.1.0.tgz", + "integrity": "sha512-SQ7Kzhh9+D+ZW9MA0zkYv3VXhIDNx+LzM6EJ+/65I3QY+enU6Itte7E5XX7EWrqLW2FN4n06GWzBnPoC3th2aQ==" }, "@istanbuljs/load-nyc-config": { "version": "1.1.0", diff --git a/package.json b/package.json index 912600026eb8e..705fb779e2ad3 100644 --- a/package.json +++ b/package.json @@ -53,6 +53,7 @@ "./package.json": "./package.json" }, "dependencies": { + "@isaacs/string-locale-compare": "^1.1.0", "@npmcli/arborist": "^2.9.0", "@npmcli/ci-detect": "^1.2.0", "@npmcli/config": "^2.3.0",