Skip to content
This repository has been archived by the owner on Jan 20, 2022. It is now read-only.

fix: use Intl.Collator for string sorting when available #324

Merged
merged 1 commit into from
Sep 28, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion lib/add-rm-pkg-deps.js
Original file line number Diff line number Diff line change
@@ -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})
Expand Down Expand Up @@ -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)
}
}

Expand Down
7 changes: 4 additions & 3 deletions lib/arborist/build-ideal-tree.js
Original file line number Diff line number Diff line change
@@ -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')
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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.
Expand Down
5 changes: 3 additions & 2 deletions lib/arborist/load-virtual.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
// mixin providing the loadVirtual method
const localeCompare = require('@isaacs/string-locale-compare')('en')

const {resolve} = require('path')

Expand Down Expand Up @@ -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
Expand Down
4 changes: 3 additions & 1 deletion lib/arborist/rebuild.js
Original file line number Diff line number Diff line change
@@ -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')
Expand All @@ -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')
Expand Down
3 changes: 2 additions & 1 deletion lib/audit-report.js
Original file line number Diff line number Diff line change
@@ -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')

Expand Down Expand Up @@ -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
Expand Down
5 changes: 3 additions & 2 deletions lib/can-place-dep.js
Original file line number Diff line number Diff line change
Expand Up @@ -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')
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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'), {
Expand Down
3 changes: 2 additions & 1 deletion lib/place-dep.js
Original file line number Diff line number Diff line change
Expand Up @@ -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')
Expand Down Expand Up @@ -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) {
Expand Down
9 changes: 5 additions & 4 deletions lib/printable.js
Original file line number Diff line number Diff line change
@@ -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')

Expand Down Expand Up @@ -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)))
}

Expand All @@ -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)]))
}
}
Expand Down
3 changes: 2 additions & 1 deletion lib/shrinkwrap.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down
9 changes: 4 additions & 5 deletions lib/vuln.js
Original file line number Diff line number Diff line change
Expand Up @@ -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')
Expand Down Expand Up @@ -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],
}
}
Expand Down
13 changes: 7 additions & 6 deletions lib/yarn-lock.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,13 +28,14 @@
// is an impenetrable 10kloc of webpack flow output, which is overkill
// for something relatively simple and tailored to Arborist's use case.

const localeCompare = require('@isaacs/string-locale-compare')('en')
const consistentResolve = require('./consistent-resolve.js')
const {dirname} = require('path')
const {breadth} = require('treeverse')

// sort a key/value object into a string of JSON stringified keys and vals
const sortKV = obj => Object.keys(obj)
.sort((a, b) => a.localeCompare(b, 'en'))
.sort(localeCompare)
.map(k => ` ${JSON.stringify(k)} ${JSON.stringify(obj[k])}`)
.join('\n')

Expand Down Expand Up @@ -170,7 +171,7 @@ class YarnLock {
toString () {
return prefix + [...new Set([...this.entries.values()])]
.map(e => e.toString())
.sort((a, b) => a.localeCompare(b, 'en')).join('\n\n') + '\n'
.sort(localeCompare).join('\n\n') + '\n'
}

fromTree (tree) {
Expand All @@ -180,15 +181,15 @@ class YarnLock {
tree,
visit: node => this.addEntryFromNode(node),
getChildren: node => [...node.children.values(), ...node.fsChildren]
.sort((a, b) => a.depth - b.depth || a.name.localeCompare(b.name, 'en')),
.sort((a, b) => a.depth - b.depth || localeCompare(a.name, b.name)),
})
return this
}

addEntryFromNode (node) {
const specs = [...node.edgesIn]
.map(e => `${node.name}@${e.spec}`)
.sort((a, b) => a.localeCompare(b, 'en'))
.sort(localeCompare)

// Note:
// yarn will do excessive duplication in a case like this:
Expand Down Expand Up @@ -321,7 +322,7 @@ class YarnLockEntry {
toString () {
// sort objects to the bottom, then alphabetical
return ([...this[_specs]]
.sort((a, b) => a.localeCompare(b, 'en'))
.sort(localeCompare)
.map(JSON.stringify).join(', ') +
':\n' +
Object.getOwnPropertyNames(this)
Expand All @@ -330,7 +331,7 @@ class YarnLockEntry {
(a, b) =>
/* istanbul ignore next - sort call order is unpredictable */
(typeof this[a] === 'object') === (typeof this[b] === 'object')
? a.localeCompare(b, 'en')
? localeCompare(a, b)
: typeof this[a] === 'object' ? 1 : -1)
.map(prop =>
typeof this[prop] !== 'object'
Expand Down
Loading