Skip to content
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
46 changes: 46 additions & 0 deletions workspaces/arborist/lib/arborist/reify.js
Original file line number Diff line number Diff line change
Expand Up @@ -1423,6 +1423,8 @@ module.exports = cls => class Reifier extends cls {
// Locations are normalized to forward slashes here because IsolatedNode/IsolatedLink locations are built with path.join, which uses backslashes on Windows.
const validKeys = new Set()
const nmDirs = new Map()
// Valid bin shim names per node_modules dir, collected from each top-level link's package.bin so the .bin sweep keeps only shims a still-linked package provides.
const binsByDir = new Map()
const NM_PREFIX = 'node_modules/'
const STORE_MARKER = '/.store/'
for (const child of this.idealTree.children.values()) {
Expand Down Expand Up @@ -1462,6 +1464,19 @@ module.exports = cls => class Reifier extends cls {
nmDirs.set(dir, set)
}
set.add(entry)

// package.bin is normalized to an object keyed by bin name; shim names are unscoped even for scoped packages.
const bin = child.package?.bin
if (bin && typeof bin === 'object') {
let binSet = binsByDir.get(dir)
if (!binSet) {
binSet = new Set()
binsByDir.set(dir, binSet)
}
for (const bn of Object.keys(bin)) {
binSet.add(bn)
}
}
}

// Determine which node_modules directories to sweep.
Expand Down Expand Up @@ -1535,7 +1550,38 @@ module.exports = cls => class Reifier extends cls {

for (const [dir, valid] of nmDirs) {
await this.#cleanOrphanedTopLevelLinks(dir, valid)
await this.#cleanStaleBinLinks(dir, binsByDir.get(dir))
}
}

// Remove stale bin shims left in node_modules/.bin after an uninstall under linked, where the diff never emits an action to drop them.
// A shim is stale when no still-linked package provides its name, or when it is a dangling symlink; matching by name handles both POSIX symlinks and Windows .cmd/.ps1 shims.
async #cleanStaleBinLinks (nmDir, validBins = new Set()) {
const binDir = resolve(nmDir, '.bin')
let names
try {
names = await readdir(binDir)
} catch {
return
}

const stale = names.filter(name => {
const base = name.replace(/\.(cmd|ps1)$/, '')
return !validBins.has(base) || !existsSync(resolve(binDir, name))
})

if (!stale.length) {
return
}

log.silly('reify', 'cleaning stale bin links', stale)
await promiseAllRejectLate(
stale.map(name =>
rm(resolve(binDir, name), { force: true })
.catch(/* istanbul ignore next -- rm with force rarely fails */
er => log.warn('cleanup', `Failed to remove stale bin link ${name}`, er))
)
)
}

// Remove node_modules/ entries that aren't represented in the ideal tree.
Expand Down
34 changes: 34 additions & 0 deletions workspaces/arborist/test/arborist/reify.js
Original file line number Diff line number Diff line change
Expand Up @@ -4252,6 +4252,40 @@ t.test('install strategy linked', async (t) => {
'packages/b should remain absent after reinstall'
)
})

t.test('removes stale .bin shims after uninstall, keeps surviving ones', async t => {
// Regression test for https://github.com/npm/cli/issues/9613
const path = t.testdir({
'package.json': JSON.stringify({ name: 'un', version: '1.0.0' }),
})
createRegistry(t, true)
const binDir = resolve(path, 'node_modules/.bin')
const rbin = resolve(binDir, 'rimraf')
const sbin = resolve(binDir, 'semver')

await reify(path, { add: ['rimraf@2.7.1', 'semver@7.3.2'], installStrategy: 'linked' })
// lstatSync throws if missing; don't assert symlink since Windows shims are regular files.
t.ok(fs.lstatSync(rbin), 'rimraf shim created')
t.ok(fs.lstatSync(sbin), 'semver shim created')

// Plant Windows shim files alongside the POSIX symlinks to cover both layouts: stale (rimraf) and surviving (semver).
const rcmd = resolve(binDir, 'rimraf.cmd')
const rps1 = resolve(binDir, 'rimraf.ps1')
const scmd = resolve(binDir, 'semver.cmd')
const sps1 = resolve(binDir, 'semver.ps1')
fs.writeFileSync(rcmd, '@echo off\r\n"%~dp0\\..\\rimraf\\bin.js" %*\r\n')
fs.writeFileSync(rps1, '& "$basedir/../rimraf/bin.js" $args\r\n')
fs.writeFileSync(scmd, '@echo off\r\n"%~dp0\\..\\semver\\bin\\semver.js" %*\r\n')
fs.writeFileSync(sps1, '& "$basedir/../semver/bin/semver.js" $args\r\n')

await reify(path, { rm: ['rimraf'], installStrategy: 'linked' })
t.throws(() => fs.lstatSync(rbin), 'stale rimraf symlink removed')
t.throws(() => fs.lstatSync(rcmd), 'stale rimraf.cmd removed')
t.throws(() => fs.lstatSync(rps1), 'stale rimraf.ps1 removed')
t.ok(fs.lstatSync(sbin), 'surviving semver shim kept')
t.ok(fs.lstatSync(scmd), 'surviving semver.cmd kept')
t.ok(fs.lstatSync(sps1), 'surviving semver.ps1 kept')
})
})

t.test('linked strategy exposes store node_modules via NODE_PATH for lifecycle scripts', async t => {
Expand Down
Loading