Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

refactor AND bugfix #5350

Merged
merged 2 commits into from
Aug 24, 2022
Merged

refactor AND bugfix #5350

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
adb7dcb
fix: inline single-use functions
wraithgar Aug 23, 2022
d30586e
fix: create links relative to the target
wraithgar Aug 24, 2022
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
131 changes: 62 additions & 69 deletions workspaces/arborist/lib/add-rm-pkg-deps.js
View file
Open in desktop
Original file line number Diff line number Diff line change
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)
}
lukekarrys marked this conversation as resolved.
Show resolved Hide resolved

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)
lukekarrys marked this conversation as resolved.
Show resolved Hide resolved
}

// Removes a subkey and warns about it if it's being replaced
Expand Down
163 changes: 58 additions & 105 deletions workspaces/arborist/lib/arborist/build-ideal-tree.js
View file
Open in desktop
Original file line number Diff line number Diff line change
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 @@ -530,85 +511,61 @@ Try using the package name instead, e.g:
this[_depsQueue].push(tree)
}

// This returns a promise because we might not have the name yet,
// and need to call pacote.manifest to find the name.
// 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 }) {
// If we have a link it will need to be added relative to the target's path
const path = tree.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 => {
// 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))
// 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.
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 tree being added to.
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 +643,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 +1187,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
Loading