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..e9a8720d7322d 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]) } } } @@ -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, @@ -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 @@ -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 } diff --git a/workspaces/arborist/tap-snapshots/test/arborist/build-ideal-tree.js.test.cjs b/workspaces/arborist/tap-snapshots/test/arborist/build-ideal-tree.js.test.cjs index 1b1e2d55da5de..42327a9db1924 100644 --- a/workspaces/arborist/tap-snapshots/test/arborist/build-ideal-tree.js.test.cjs +++ b/workspaces/arborist/tap-snapshots/test/arborist/build-ideal-tree.js.test.cjs @@ -1470,6 +1470,207 @@ ArboristNode { } ` +exports[`test/arborist/build-ideal-tree.js TAP add one workspace to another > tree with workspace a added to workspace c 1`] = ` +ArboristNode { + "children": Map { + "a" => ArboristLink { + "edgesIn": Set { + EdgeIn { + "from": "", + "name": "a", + "spec": "file:{CWD}/test/fixtures/workspaces-not-root/packages/a", + "type": "workspace", + }, + EdgeIn { + "from": "packages/c", + "name": "a", + "spec": "file:../a", + "type": "prod", + }, + }, + "isWorkspace": true, + "location": "node_modules/a", + "name": "a", + "path": "{CWD}/test/fixtures/workspaces-not-root/node_modules/a", + "realpath": "{CWD}/test/fixtures/workspaces-not-root/packages/a", + "resolved": "file:../packages/a", + "target": ArboristNode { + "location": "packages/a", + }, + "version": "1.2.3", + }, + "abbrev" => ArboristNode { + "edgesIn": Set { + EdgeIn { + "from": "", + "name": "abbrev", + "spec": "*", + "type": "prod", + }, + EdgeIn { + "from": "packages/b", + "name": "abbrev", + "spec": "*", + "type": "prod", + }, + EdgeIn { + "from": "packages/c", + "name": "abbrev", + "spec": "*", + "type": "prod", + }, + }, + "location": "node_modules/abbrev", + "name": "abbrev", + "path": "{CWD}/test/fixtures/workspaces-not-root/node_modules/abbrev", + "resolved": "https://registry.npmjs.org/abbrev/-/abbrev-1.1.1.tgz", + "version": "1.1.1", + }, + "b" => ArboristLink { + "edgesIn": Set { + EdgeIn { + "from": "", + "name": "b", + "spec": "file:{CWD}/test/fixtures/workspaces-not-root/packages/b", + "type": "workspace", + }, + }, + "isWorkspace": true, + "location": "node_modules/b", + "name": "b", + "path": "{CWD}/test/fixtures/workspaces-not-root/node_modules/b", + "realpath": "{CWD}/test/fixtures/workspaces-not-root/packages/b", + "resolved": "file:../packages/b", + "target": ArboristNode { + "location": "packages/b", + }, + "version": "1.2.3", + }, + "c" => ArboristLink { + "edgesIn": Set { + EdgeIn { + "from": "", + "name": "c", + "spec": "file:{CWD}/test/fixtures/workspaces-not-root/packages/c", + "type": "workspace", + }, + }, + "isWorkspace": true, + "location": "node_modules/c", + "name": "c", + "path": "{CWD}/test/fixtures/workspaces-not-root/node_modules/c", + "realpath": "{CWD}/test/fixtures/workspaces-not-root/packages/c", + "resolved": "file:../packages/c", + "target": ArboristNode { + "location": "packages/c", + }, + "version": "1.2.3", + }, + "wrappy" => ArboristNode { + "edgesIn": Set { + EdgeIn { + "from": "", + "name": "wrappy", + "spec": "1.0.0", + "type": "prod", + }, + }, + "location": "node_modules/wrappy", + "name": "wrappy", + "path": "{CWD}/test/fixtures/workspaces-not-root/node_modules/wrappy", + "resolved": "https://registry.npmjs.org/wrappy/-/wrappy-1.0.0.tgz", + "version": "1.0.0", + }, + }, + "edgesOut": Map { + "a" => EdgeOut { + "name": "a", + "spec": "file:{CWD}/test/fixtures/workspaces-not-root/packages/a", + "to": "node_modules/a", + "type": "workspace", + }, + "abbrev" => EdgeOut { + "name": "abbrev", + "spec": "*", + "to": "node_modules/abbrev", + "type": "prod", + }, + "b" => EdgeOut { + "name": "b", + "spec": "file:{CWD}/test/fixtures/workspaces-not-root/packages/b", + "to": "node_modules/b", + "type": "workspace", + }, + "c" => EdgeOut { + "name": "c", + "spec": "file:{CWD}/test/fixtures/workspaces-not-root/packages/c", + "to": "node_modules/c", + "type": "workspace", + }, + "wrappy" => EdgeOut { + "name": "wrappy", + "spec": "1.0.0", + "to": "node_modules/wrappy", + "type": "prod", + }, + }, + "fsChildren": Set { + ArboristNode { + "isWorkspace": true, + "location": "packages/a", + "name": "a", + "path": "{CWD}/test/fixtures/workspaces-not-root/packages/a", + "version": "1.2.3", + }, + ArboristNode { + "edgesOut": Map { + "abbrev" => EdgeOut { + "name": "abbrev", + "spec": "*", + "to": "node_modules/abbrev", + "type": "prod", + }, + }, + "isWorkspace": true, + "location": "packages/b", + "name": "b", + "path": "{CWD}/test/fixtures/workspaces-not-root/packages/b", + "version": "1.2.3", + }, + ArboristNode { + "edgesOut": Map { + "a" => EdgeOut { + "name": "a", + "spec": "file:../a", + "to": "node_modules/a", + "type": "prod", + }, + "abbrev" => EdgeOut { + "name": "abbrev", + "spec": "*", + "to": "node_modules/abbrev", + "type": "prod", + }, + }, + "isWorkspace": true, + "location": "packages/c", + "name": "c", + "path": "{CWD}/test/fixtures/workspaces-not-root/packages/c", + "version": "1.2.3", + }, + }, + "isProjectRoot": true, + "location": "", + "name": "workspaces-not-root", + "path": "{CWD}/test/fixtures/workspaces-not-root", + "workspaces": Map { + "a" => "packages/a", + "b" => "packages/b", + "c" => "packages/c", + }, +} +` + exports[`test/arborist/build-ideal-tree.js TAP add packages to workspaces, not root > tree with abbrev removed from a and b 1`] = ` ArboristNode { "children": Map { diff --git a/workspaces/arborist/test/arborist/build-ideal-tree.js b/workspaces/arborist/test/arborist/build-ideal-tree.js index 87783086b65c3..0f7c5fecf4fd9 100644 --- a/workspaces/arborist/test/arborist/build-ideal-tree.js +++ b/workspaces/arborist/test/arborist/build-ideal-tree.js @@ -2674,6 +2674,19 @@ t.test('add packages to workspaces, not root', async t => { t.matchSnapshot(printTree(rmTree), 'tree with abbrev removed from a and b') }) +t.test('add one workspace to another', async t => { + const path = resolve(__dirname, '../fixtures/workspaces-not-root') + const packageA = resolve(path, 'packages/a') + + const addTree = await buildIdeal(path, { + add: [packageA], + workspaces: ['c'], + }) + const c = addTree.children.get('c').target + t.match(c.edgesOut.get('a'), { spec: 'file:../a' }) + t.matchSnapshot(printTree(addTree), 'tree with workspace a added to workspace c') +}) + t.test('workspace error handling', async t => { const path = t.testdir({ 'package.json': JSON.stringify({