Skip to content

Commit

Permalink
fix: inline single-use functions
Browse files Browse the repository at this point in the history
  • Loading branch information
@wraithgar @lukekarrys
wraithgar authored and lukekarrys committed Aug 24, 2022
1 parent 3569094 commit ea5e3a3
Show file tree
Hide file tree
Showing 2 changed files with 112 additions and 167 deletions.
131 changes: 62 additions & 69 deletions workspaces/arborist/lib/add-rm-pkg-deps.js
View file
Expand Up @@ -4,8 +4,67 @@ const log = require('proc-log')
const localeCompare = require('@isaacs/string-locale-compare')('en')

const add = ({ pkg, add, saveBundle, saveType }) => {
for (const spec of add) {
addSingle({ pkg, spec, saveBundle, saveType })
for (const { name, rawSpec } of add) {
// if the user does not give us a type, we infer which type(s)
// to keep based on the same order of priority we do when
// building the tree as defined in the _loadDeps method of
// the node class.
if (!saveType) {
saveType = inferSaveType(pkg, name)
}

if (saveType === 'prod') {
// a production dependency can only exist as production (rpj ensures it
// doesn't coexist w/ optional)
deleteSubKey(pkg, 'devDependencies', name, 'dependencies')
deleteSubKey(pkg, 'peerDependencies', name, 'dependencies')
} else if (saveType === 'dev') {
// a dev dependency may co-exist as peer, or optional, but not production
deleteSubKey(pkg, 'dependencies', name, 'devDependencies')
} else if (saveType === 'optional') {
// an optional dependency may co-exist as dev (rpj ensures it doesn't
// coexist w/ prod)
deleteSubKey(pkg, 'peerDependencies', name, 'optionalDependencies')
} else { // peer or peerOptional is all that's left
// a peer dependency may coexist as dev
deleteSubKey(pkg, 'dependencies', name, 'peerDependencies')
deleteSubKey(pkg, 'optionalDependencies', name, 'peerDependencies')
}

const depType = saveTypeMap.get(saveType)

pkg[depType] = pkg[depType] || {}
if (rawSpec !== '' || pkg[depType][name] === undefined) {
pkg[depType][name] = rawSpec || '*'
}
if (saveType === 'optional') {
// Affordance for previous npm versions that require this behaviour
pkg.dependencies = pkg.dependencies || {}
pkg.dependencies[name] = pkg.optionalDependencies[name]
}

if (saveType === 'peer' || saveType === 'peerOptional') {
const pdm = pkg.peerDependenciesMeta || {}
if (saveType === 'peer' && pdm[name] && pdm[name].optional) {
pdm[name].optional = false
} else if (saveType === 'peerOptional') {
pdm[name] = pdm[name] || {}
pdm[name].optional = true
pkg.peerDependenciesMeta = pdm
}
// peerDeps are often also a devDep, so that they can be tested when
// using package managers that don't auto-install peer deps
if (pkg.devDependencies && pkg.devDependencies[name] !== undefined) {
pkg.devDependencies[name] = pkg.peerDependencies[name]
}
}

if (saveBundle && saveType !== 'peer' && saveType !== 'peerOptional') {
// keep it sorted, keep it unique
const bd = new Set(pkg.bundleDependencies || [])
bd.add(name)
pkg.bundleDependencies = [...bd].sort(localeCompare)
}
}

return pkg
Expand All @@ -21,71 +80,6 @@ const saveTypeMap = new Map([
['peer', 'peerDependencies'],
])

const addSingle = ({ pkg, spec, saveBundle, saveType }) => {
const { name, rawSpec } = spec

// if the user does not give us a type, we infer which type(s)
// to keep based on the same order of priority we do when
// building the tree as defined in the _loadDeps method of
// the node class.
if (!saveType) {
saveType = inferSaveType(pkg, spec.name)
}

if (saveType === 'prod') {
// a production dependency can only exist as production (rpj ensures it
// doesn't coexist w/ optional)
deleteSubKey(pkg, 'devDependencies', name, 'dependencies')
deleteSubKey(pkg, 'peerDependencies', name, 'dependencies')
} else if (saveType === 'dev') {
// a dev dependency may co-exist as peer, or optional, but not production
deleteSubKey(pkg, 'dependencies', name, 'devDependencies')
} else if (saveType === 'optional') {
// an optional dependency may co-exist as dev (rpj ensures it doesn't
// coexist w/ prod)
deleteSubKey(pkg, 'peerDependencies', name, 'optionalDependencies')
} else { // peer or peerOptional is all that's left
// a peer dependency may coexist as dev
deleteSubKey(pkg, 'dependencies', name, 'peerDependencies')
deleteSubKey(pkg, 'optionalDependencies', name, 'peerDependencies')
}

const depType = saveTypeMap.get(saveType)

pkg[depType] = pkg[depType] || {}
if (rawSpec !== '' || pkg[depType][name] === undefined) {
pkg[depType][name] = rawSpec || '*'
}
if (saveType === 'optional') {
// Affordance for previous npm versions that require this behaviour
pkg.dependencies = pkg.dependencies || {}
pkg.dependencies[name] = pkg.optionalDependencies[name]
}

if (saveType === 'peer' || saveType === 'peerOptional') {
const pdm = pkg.peerDependenciesMeta || {}
if (saveType === 'peer' && pdm[name] && pdm[name].optional) {
pdm[name].optional = false
} else if (saveType === 'peerOptional') {
pdm[name] = pdm[name] || {}
pdm[name].optional = true
pkg.peerDependenciesMeta = pdm
}
// peerDeps are often also a devDep, so that they can be tested when
// using package managers that don't auto-install peer deps
if (pkg.devDependencies && pkg.devDependencies[name] !== undefined) {
pkg.devDependencies[name] = pkg.peerDependencies[name]
}
}

if (saveBundle && saveType !== 'peer' && saveType !== 'peerOptional') {
// keep it sorted, keep it unique
const bd = new Set(pkg.bundleDependencies || [])
bd.add(spec.name)
pkg.bundleDependencies = [...bd].sort(localeCompare)
}
}

// Finds where the package is already in the spec and infers saveType from that
const inferSaveType = (pkg, name) => {
for (const saveType of saveTypeMap.keys()) {
Expand All @@ -103,9 +97,8 @@ const inferSaveType = (pkg, name) => {
return 'prod'
}

const { hasOwnProperty } = Object.prototype
const hasSubKey = (pkg, depType, name) => {
return pkg[depType] && hasOwnProperty.call(pkg[depType], name)
return pkg[depType] && Object.prototype.hasOwnProperty.call(pkg[depType], name)
}

// Removes a subkey and warns about it if it's being replaced
Expand Down
148 changes: 50 additions & 98 deletions workspaces/arborist/lib/arborist/build-ideal-tree.js
View file
Expand Up @@ -81,18 +81,11 @@ const _linkNodes = Symbol('linkNodes')
const _follow = Symbol('follow')
const _globalStyle = Symbol('globalStyle')
const _globalRootNode = Symbol('globalRootNode')
const _isVulnerable = Symbol.for('isVulnerable')
const _usePackageLock = Symbol.for('usePackageLock')
const _rpcache = Symbol.for('realpathCache')
const _stcache = Symbol.for('statCache')
const _updateFilePath = Symbol('updateFilePath')
const _followSymlinkPath = Symbol('followSymlinkPath')
const _getRelpathSpec = Symbol('getRelpathSpec')
const _retrieveSpecName = Symbol('retrieveSpecName')
const _strictPeerDeps = Symbol('strictPeerDeps')
const _checkEngineAndPlatform = Symbol('checkEngineAndPlatform')
const _checkEngine = Symbol('checkEngine')
const _checkPlatform = Symbol('checkPlatform')
const _virtualRoots = Symbol('virtualRoots')
const _virtualRoot = Symbol('virtualRoot')
const _includeWorkspaceRoot = Symbol.for('includeWorkspaceRoot')
Expand Down Expand Up @@ -228,34 +221,22 @@ module.exports = cls => class IdealTreeBuilder extends cls {
}

async [_checkEngineAndPlatform] () {
const { engineStrict, npmVersion, nodeVersion } = this.options
for (const node of this.idealTree.inventory.values()) {
if (!node.optional) {
this[_checkEngine](node)
this[_checkPlatform](node)
}
}
}

[_checkPlatform] (node) {
checkPlatform(node.package, this[_force])
}

[_checkEngine] (node) {
const { engineStrict, npmVersion, nodeVersion } = this.options
const c = () =>
checkEngine(node.package, npmVersion, nodeVersion, this[_force])

if (engineStrict) {
c()
} else {
try {
c()
} catch (er) {
log.warn(er.code, er.message, {
package: er.pkgid,
required: er.required,
current: er.current,
})
try {
checkEngine(node.package, npmVersion, nodeVersion, this[_force])
} catch (err) {
if (engineStrict) {
throw err
}
log.warn(err.code, err.message, {
package: err.pkgid,
required: err.required,
current: err.current,
})
}
checkPlatform(node.package, this[_force])
}
}
}
Expand Down Expand Up @@ -533,82 +514,57 @@ Try using the package name instead, e.g:
// This returns a promise because we might not have the name yet,
// and need to call pacote.manifest to find the name.
async [_add] (tree, { add, saveType = null, saveBundle = false }) {
const path = this.idealTree.target.path
// get the name for each of the specs in the list.
// ie, doing `foo@bar` we just return foo
// but if it's a url or git, we don't know the name until we
// fetch it and look in its manifest.
const resolvedAdd = await Promise.all(add.map(async rawSpec => {
await Promise.all(add.map(async rawSpec => {
// We do NOT provide the path to npa here, because user-additions
// need to be resolved relative to the CWD the user is in.
const spec = await this[_retrieveSpecName](npa(rawSpec))
.then(spec => this[_updateFilePath](spec))
.then(spec => this[_followSymlinkPath](spec))
let spec = npa(rawSpec)

// if it's just @'' then we reload whatever's there, or get latest
// if it's an explicit tag, we need to install that specific tag version
const isTag = spec.rawSpec && spec.type === 'tag'

// look up the names of file/directory/git specs
if (!spec.name || isTag) {
const mani = await pacote.manifest(spec, { ...this.options })
if (isTag) {
// translate tag to a version
spec = npa(`${mani.name}@${mani.version}`)
}
spec.name = mani.name
}

const { name } = spec
if (spec.type === 'file') {
spec = npa(`file:${relpath(path, spec.fetchSpec).replace(/#/g, '%23')}`, path)
spec.name = name
} else if (spec.type === 'directory') {
try {
const real = await realpath(spec.fetchSpec, this[_rpcache], this[_stcache])
spec = npa(`file:${relpath(path, real).replace(/#/g, '%23')}`, path)
spec.name = name
} catch {
// TODO: create synthetic test case to simulate realpath failure
}
}
spec.tree = tree
return spec
this[_resolvedAdd].push(spec)
}))
this[_resolvedAdd].push(...resolvedAdd)
// now resolvedAdd is a list of spec objects with names.

// now this._resolvedAdd is a list of spec objects with names.
// find a home for each of them!
addRmPkgDeps.add({
pkg: tree.package,
add: resolvedAdd,
add: this[_resolvedAdd],
saveBundle,
saveType,
path: this.path,
})
}

async [_retrieveSpecName] (spec) {
// if it's just @'' then we reload whatever's there, or get latest
// if it's an explicit tag, we need to install that specific tag version
const isTag = spec.rawSpec && spec.type === 'tag'

if (spec.name && !isTag) {
return spec
}

const mani = await pacote.manifest(spec, { ...this.options })
// if it's a tag type, then we need to run it down to an actual version
if (isTag) {
return npa(`${mani.name}@${mani.version}`)
}

spec.name = mani.name
return spec
}

async [_updateFilePath] (spec) {
if (spec.type === 'file') {
return this[_getRelpathSpec](spec, spec.fetchSpec)
}

return spec
}

async [_followSymlinkPath] (spec) {
if (spec.type === 'directory') {
const real = await (
realpath(spec.fetchSpec, this[_rpcache], this[_stcache])
// TODO: create synthetic test case to simulate realpath failure
.catch(/* istanbul ignore next */() => null)
)

return this[_getRelpathSpec](spec, real)
}
return spec
}

[_getRelpathSpec] (spec, filepath) {
/* istanbul ignore else - should also be covered by realpath failure */
if (filepath) {
const { name } = spec
const tree = this.idealTree.target
spec = npa(`file:${relpath(tree.path, filepath).replace(/#/g, '%23')}`, tree.path)
spec.name = name
}
return spec
}

// TODO: provide a way to fix bundled deps by exposing metadata about
// what's in the bundle at each published manifest. Without that, we
// can't possibly fix bundled deps without breaking a ton of other stuff,
Expand Down Expand Up @@ -686,10 +642,6 @@ Try using the package name instead, e.g:
}
}

[_isVulnerable] (node) {
return this.auditReport && this.auditReport.isVulnerable(node)
}

[_avoidRange] (name) {
if (!this.auditReport) {
return null
Expand Down Expand Up @@ -1234,7 +1186,7 @@ This is a one-time fix-up, please be patient...
}

// fixing a security vulnerability with this package, problem
if (this[_isVulnerable](edge.to)) {
if (this.auditReport && this.auditReport.isVulnerable(edge.to)) {
return true
}

Expand Down

0 comments on commit ea5e3a3

Please sign in to comment.