From ea5e3a319a7c1b5f7f2a66284227a34b79b2c831 Mon Sep 17 00:00:00 2001 From: Gar Date: Tue, 23 Aug 2022 08:23:20 -0700 Subject: [PATCH] fix: inline single-use functions --- workspaces/arborist/lib/add-rm-pkg-deps.js | 131 ++++++++-------- .../arborist/lib/arborist/build-ideal-tree.js | 148 ++++++------------ 2 files changed, 112 insertions(+), 167 deletions(-) diff --git a/workspaces/arborist/lib/add-rm-pkg-deps.js b/workspaces/arborist/lib/add-rm-pkg-deps.js index f59df359e9456..7b43c38e2492b 100644 --- a/workspaces/arborist/lib/add-rm-pkg-deps.js +++ b/workspaces/arborist/lib/add-rm-pkg-deps.js @@ -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 @@ -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()) { @@ -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 diff --git a/workspaces/arborist/lib/arborist/build-ideal-tree.js b/workspaces/arborist/lib/arborist/build-ideal-tree.js index 31a4e8c821a8c..64d72bedacb1c 100644 --- a/workspaces/arborist/lib/arborist/build-ideal-tree.js +++ b/workspaces/arborist/lib/arborist/build-ideal-tree.js @@ -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') @@ -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]) } } } @@ -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, @@ -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 @@ -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 }