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.

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

fix: Arborist code cleanup #7237

Merged
merged 20 commits into from
Feb 28, 2024
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
51 changes: 0 additions & 51 deletions workspaces/arborist/lib/arborist/audit.js

This file was deleted.

102 changes: 56 additions & 46 deletions workspaces/arborist/lib/arborist/build-ideal-tree.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,22 +38,15 @@ const resetDepFlags = require('../reset-dep-flags.js')
// them with unit tests and reuse them across mixins
const _updateAll = Symbol.for('updateAll')
const _flagsSuspect = Symbol.for('flagsSuspect')
const _workspaces = Symbol.for('workspaces')
const _setWorkspaces = Symbol.for('setWorkspaces')
const _updateNames = Symbol.for('updateNames')
const _resolvedAdd = Symbol.for('resolvedAdd')
const _usePackageLock = Symbol.for('usePackageLock')
const _rpcache = Symbol.for('realpathCache')
const _stcache = Symbol.for('statCache')
const _includeWorkspaceRoot = Symbol.for('includeWorkspaceRoot')

// exposed symbol for unit testing the placeDep method directly
const _peerSetSource = Symbol.for('peerSetSource')

// used by Reify mixin
const _force = Symbol.for('force')
const _global = Symbol.for('global')
const _idealTreePrune = Symbol.for('idealTreePrune')
const _addNodeToTrashList = Symbol.for('addNodeToTrashList')

// Push items in, pop them sorted by depth and then path
// Sorts physically shallower deps up to the front of the queue, because
Expand Down Expand Up @@ -117,6 +110,10 @@ module.exports = cls => class IdealTreeBuilder extends cls {
#loadFailures = new Set()
#manifests = new Map()
#mutateTree = false
// a map of each module in a peer set to the thing that depended on
// that set of peers in the first place. Use a WeakMap so that we
// don't hold onto references for nodes that are garbage collected.
#peerSetSource = new WeakMap()
#preferDedupe = false
#prune
#strictPeerDeps
Expand All @@ -131,45 +128,33 @@ module.exports = cls => class IdealTreeBuilder extends cls {

const {
follow = false,
force = false,
global = false,
installStrategy = 'hoisted',
idealTree = null,
includeWorkspaceRoot = false,
installLinks = false,
legacyPeerDeps = false,
packageLock = true,
strictPeerDeps = false,
workspaces = [],
workspaces,
global,
} = options

this[_workspaces] = workspaces || []
this[_force] = !!force
this.#strictPeerDeps = !!strictPeerDeps

this.idealTree = idealTree
this.installLinks = installLinks
this.legacyPeerDeps = legacyPeerDeps

this[_usePackageLock] = packageLock
this[_global] = !!global
this.#installStrategy = global ? 'shallow' : installStrategy
this.#follow = !!follow

if (this[_workspaces].length && this[_global]) {
if (workspaces?.length && global) {
throw new Error('Cannot operate on workspaces in global mode')
}

this[_updateAll] = false
this[_updateNames] = []
this[_resolvedAdd] = []

// a map of each module in a peer set to the thing that depended on
// that set of peers in the first place. Use a WeakMap so that we
// don't hold onto references for nodes that are garbage collected.
this[_peerSetSource] = new WeakMap()

this[_includeWorkspaceRoot] = includeWorkspaceRoot
}

get explicitRequests () {
Expand All @@ -196,7 +181,7 @@ module.exports = cls => class IdealTreeBuilder extends cls {

process.emit('time', 'idealTree')

if (!options.add && !options.rm && !options.update && this[_global]) {
if (!options.add && !options.rm && !options.update && this.options.global) {
throw new Error('global requires add, rm, or update option')
}

Expand Down Expand Up @@ -232,7 +217,7 @@ module.exports = cls => class IdealTreeBuilder extends cls {
for (const node of this.idealTree.inventory.values()) {
if (!node.optional) {
try {
checkEngine(node.package, npmVersion, nodeVersion, this[_force])
checkEngine(node.package, npmVersion, nodeVersion, this.options.force)
} catch (err) {
if (engineStrict) {
throw err
Expand All @@ -243,7 +228,7 @@ module.exports = cls => class IdealTreeBuilder extends cls {
current: err.current,
})
}
checkPlatform(node.package, this[_force])
checkPlatform(node.package, this.options.force)
}
}
}
Expand Down Expand Up @@ -295,7 +280,7 @@ module.exports = cls => class IdealTreeBuilder extends cls {
async #initTree () {
process.emit('time', 'idealTree:init')
let root
if (this[_global]) {
if (this.options.global) {
root = await this.#globalRootNode()
} else {
try {
Expand All @@ -313,7 +298,7 @@ module.exports = cls => class IdealTreeBuilder extends cls {
// When updating all, we load the shrinkwrap, but don't bother
// to build out the full virtual tree from it, since we'll be
// reconstructing it anyway.
.then(root => this[_global] ? root
.then(root => this.options.global ? root
: !this[_usePackageLock] || this[_updateAll]
? Shrinkwrap.reset({
path: this.path,
Expand All @@ -329,7 +314,7 @@ module.exports = cls => class IdealTreeBuilder extends cls {
// Load on a new Arborist object, so the Nodes aren't the same,
// or else it'll get super confusing when we change them!
.then(async root => {
if ((!this[_updateAll] && !this[_global] && !root.meta.loadedFromDisk) || (this[_global] && this[_updateNames].length)) {
if ((!this[_updateAll] && !this.options.global && !root.meta.loadedFromDisk) || (this.options.global && this[_updateNames].length)) {
await new this.constructor(this.options).loadActual({ root })
const tree = root.target
// even though we didn't load it from a package-lock.json FILE,
Expand Down Expand Up @@ -408,7 +393,7 @@ module.exports = cls => class IdealTreeBuilder extends cls {
devOptional: false,
peer: false,
optional: false,
global: this[_global],
global: this.options.global,
installLinks: this.installLinks,
legacyPeerDeps: this.legacyPeerDeps,
loadOverrides: true,
Expand All @@ -423,7 +408,7 @@ module.exports = cls => class IdealTreeBuilder extends cls {
devOptional: false,
peer: false,
optional: false,
global: this[_global],
global: this.options.global,
installLinks: this.installLinks,
legacyPeerDeps: this.legacyPeerDeps,
root,
Expand All @@ -438,11 +423,11 @@ module.exports = cls => class IdealTreeBuilder extends cls {
process.emit('time', 'idealTree:userRequests')
const tree = this.idealTree.target

if (!this[_workspaces].length) {
if (!this.options.workspaces.length) {
await this.#applyUserRequestsToNode(tree, options)
} else {
const nodes = this.workspaceNodes(tree, this[_workspaces])
if (this[_includeWorkspaceRoot]) {
const nodes = this.workspaceNodes(tree, this.options.workspaces)
if (this.options.includeWorkspaceRoot) {
nodes.push(tree)
}
const appliedRequests = nodes.map(
Expand All @@ -458,14 +443,14 @@ module.exports = cls => class IdealTreeBuilder extends cls {
// If we have a list of package names to update, and we know it's
// going to update them wherever they are, add any paths into those
// named nodes to the buildIdealTree queue.
if (!this[_global] && this[_updateNames].length) {
if (!this.options.global && this[_updateNames].length) {
this.#queueNamedUpdates()
}

// global updates only update the globalTop nodes, but we need to know
// that they're there, and not reinstall the world unnecessarily.
const globalExplicitUpdateNames = []
if (this[_global] && (this[_updateAll] || this[_updateNames].length)) {
if (this.options.global && (this[_updateAll] || this[_updateNames].length)) {
const nm = resolve(this.path, 'node_modules')
const paths = await readdirScoped(nm).catch(() => [])
for (const p of paths) {
Expand Down Expand Up @@ -510,7 +495,7 @@ module.exports = cls => class IdealTreeBuilder extends cls {
// triggers a refresh of all edgesOut. this has to be done BEFORE
// adding the edges to explicitRequests, because the package setter
// resets all edgesOut.
if (add && add.length || rm && rm.length || this[_global]) {
if (add && add.length || rm && rm.length || this.options.global) {
tree.package = tree.package
}

Expand Down Expand Up @@ -616,7 +601,7 @@ module.exports = cls => class IdealTreeBuilder extends cls {
//
// XXX: how to handle top nodes that aren't the root? Maybe the report
// just tells the user to cd into that directory and fix it?
if (this[_force] && this.auditReport && this.auditReport.topVulns.size) {
if (this.options.force && this.auditReport && this.auditReport.topVulns.size) {
options.add = options.add || []
options.rm = options.rm || []
const nodesTouched = new Set()
Expand Down Expand Up @@ -900,7 +885,7 @@ This is a one-time fix-up, please be patient...
// dep if allowed.

const tasks = []
const peerSource = this[_peerSetSource].get(node) || node
const peerSource = this.#peerSetSource.get(node) || node
for (const edge of this.#problemEdges(node)) {
if (edge.peerConflicted) {
continue
Expand Down Expand Up @@ -958,7 +943,7 @@ This is a one-time fix-up, please be patient...

auditReport: this.auditReport,
explicitRequest: this.#explicitRequests.has(edge),
force: this[_force],
force: this.options.force,
installLinks: this.installLinks,
installStrategy: this.#installStrategy,
legacyPeerDeps: this.legacyPeerDeps,
Expand Down Expand Up @@ -1099,13 +1084,13 @@ This is a one-time fix-up, please be patient...

// keep track of the thing that caused this node to be included.
const src = parent.sourceReference
this[_peerSetSource].set(node, src)
this.#peerSetSource.set(node, src)

// do not load the peers along with the set if this is a global top pkg
// otherwise we'll be tempted to put peers as other top-level installed
// things, potentially clobbering what's there already, which is not
// what we want. the missing edges will be picked up on the next pass.
if (this[_global] && edge.from.isProjectRoot) {
if (this.options.global && edge.from.isProjectRoot) {
return node
}

Expand Down Expand Up @@ -1328,7 +1313,7 @@ This is a one-time fix-up, please be patient...
const parentEdge = node.parent.edgesOut.get(edge.name)
const { isProjectRoot, isWorkspace } = node.parent.sourceReference
const isMine = isProjectRoot || isWorkspace
const conflictOK = this[_force] || !isMine && !this.#strictPeerDeps
const conflictOK = this.options.force || !isMine && !this.#strictPeerDeps

if (!edge.to) {
if (!parentEdge) {
Expand Down Expand Up @@ -1415,7 +1400,7 @@ This is a one-time fix-up, please be patient...
currentEdge: currentEdge ? currentEdge.explain() : null,
edge: edge.explain(),
strictPeerDeps: this.#strictPeerDeps,
force: this[_force],
force: this.options.force,
}
}

Expand Down Expand Up @@ -1503,7 +1488,7 @@ This is a one-time fix-up, please be patient...
// otherwise, don't bother.
const needPrune = metaFromDisk && (mutateTree || flagsSuspect)
if (this.#prune && needPrune) {
this[_idealTreePrune]()
this.#idealTreePrune()
for (const node of this.idealTree.inventory.values()) {
if (node.extraneous) {
node.parent = null
Expand All @@ -1514,7 +1499,7 @@ This is a one-time fix-up, please be patient...
process.emit('timeEnd', 'idealTree:fixDepFlags')
}

[_idealTreePrune] () {
#idealTreePrune () {
for (const node of this.idealTree.inventory.values()) {
if (node.extraneous) {
node.parent = null
Expand All @@ -1534,4 +1519,29 @@ This is a one-time fix-up, please be patient...
}
}
}

async prune (options = {}) {
// allow the user to set options on the ctor as well.
// XXX: deprecate separate method options objects.
options = { ...this.options, ...options }

await this.buildIdealTree(options)

this.#idealTreePrune()

if (!this.options.workspacesEnabled) {
const excludeNodes = this.excludeWorkspacesDependencySet(this.idealTree)
for (const node of this.idealTree.inventory.values()) {
if (
node.parent !== null
&& !node.isProjectRoot
&& !excludeNodes.has(node)
) {
this[_addNodeToTrashList](node)
}
}
}

return this.reify(options)
}
}
19 changes: 0 additions & 19 deletions workspaces/arborist/lib/arborist/deduper.js

This file was deleted.

Loading
Loading