Skip to content
Open
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
42 changes: 40 additions & 2 deletions workspaces/arborist/lib/arborist/reify.js
Original file line number Diff line number Diff line change
Expand Up @@ -1322,9 +1322,27 @@ module.exports = cls => class Reifier extends cls {
const nmDir = resolve(this.path, 'node_modules')
const storeDir = resolve(nmDir, '.store')

// Enumerate on-disk store entries as full keys, descending one level into each @scope directory because scoped keys nest as .store/@scope/pkg@version-hash.
let entries
try {
entries = await readdir(storeDir)
const topLevel = await readdir(storeDir, { withFileTypes: true })
entries = []
for (const ent of topLevel) {
if (ent.name.startsWith('@')) {
let scoped
try {
scoped = await readdir(resolve(storeDir, ent.name))
} catch {
/* istanbul ignore next -- readdir of an entry we just listed should not fail */
continue
}
for (const name of scoped) {
entries.push(`${ent.name}/${name}`)
}
} else {
entries.push(ent.name)
}
}
} catch {
entries = null
}
Expand All @@ -1340,7 +1358,10 @@ module.exports = cls => class Reifier extends cls {
for (const child of this.idealTree.children.values()) {
const loc = child.location.replace(/\\/g, '/')
if (child.isInStore) {
const key = loc.split('/')[2]
// Store location is node_modules/.store/{key}/node_modules/{pkg}.
// For a scoped package the key is @scope/pkg@version-hash, which spans two path segments, so reconstruct both instead of taking only the scope.
const parts = loc.split('/')
const key = parts[2].startsWith('@') ? `${parts[2]}/${parts[3]}` : parts[2]
validKeys.add(key)
continue
}
Expand Down Expand Up @@ -1422,6 +1443,23 @@ module.exports = cls => class Reifier extends cls {
er => log.warn('cleanup', `Failed to remove orphaned store entry ${e}`, er))
)
)
// Removing the last scoped orphan under a scope leaves an empty @scope directory behind, so prune any scope directory that is now empty.
const scopes = new Set(
orphaned.filter(e => e.startsWith('@')).map(e => e.split('/')[0])
)
await promiseAllRejectLate(
[...scopes].map(async scope => {
const scopeDir = resolve(storeDir, scope)
try {
const remaining = await readdir(scopeDir)
if (!remaining.length) {
await rm(scopeDir, { recursive: true, force: true })
}
} catch {
/* istanbul ignore next -- readdir of a scope dir we just listed should not fail */
}
})
)
}
}

Expand Down
79 changes: 79 additions & 0 deletions workspaces/arborist/test/isolated-mode.js
Original file line number Diff line number Diff line change
Expand Up @@ -1760,6 +1760,85 @@ tap.test('orphaned store entries are cleaned up on dependency update', async t =
'old which@1.0.0 store entry is removed after update')
})

tap.test('orphaned scoped store entries are cleaned up on dependency update', async t => {
// https://github.com/npm/cli/issues/9440 — a scoped store key spans two path segments (.store/@scope/pkg@version-hash), so the single-segment orphan cleanup never swept the stale entry.
const graph = {
registry: [
{ name: '@scope/which', version: '1.0.0', dependencies: { isexe: '^1.0.0' } },
{ name: '@scope/which', version: '2.0.0', dependencies: { isexe: '^1.0.0' } },
{ name: 'isexe', version: '1.0.0' },
],
root: {
name: 'myproject',
version: '1.0.0',
dependencies: { '@scope/which': '1.0.0' },
},
}
const { dir, registry } = await getRepo(graph)
const cache = fs.mkdtempSync(`${getTempDir()}/test-`)
const storeDir = path.join(dir, 'node_modules', '.store')
const scopeDir = path.join(storeDir, '@scope')

// First install — @scope/which@1.0.0
const arb1 = new Arborist({ path: dir, registry, packumentCache: new Map(), cache })
await arb1.reify({ installStrategy: 'linked' })

const entriesAfterV1 = fs.readdirSync(scopeDir)
t.ok(entriesAfterV1.some(e => e.startsWith('which@1.0.0-')),
'store has @scope/which@1.0.0 entry after first install')

// Update package.json to depend on @scope/which@2.0.0
const pkgPath = path.join(dir, 'package.json')
const pkg = JSON.parse(fs.readFileSync(pkgPath, 'utf8'))
pkg.dependencies['@scope/which'] = '2.0.0'
fs.writeFileSync(pkgPath, JSON.stringify(pkg))

// Second install — @scope/which@2.0.0
const arb2 = new Arborist({ path: dir, registry, packumentCache: new Map(), cache })
await arb2.reify({ installStrategy: 'linked' })

const entriesAfterV2 = fs.readdirSync(scopeDir)
t.ok(entriesAfterV2.some(e => e.startsWith('which@2.0.0-')),
'store has @scope/which@2.0.0 entry after update')
t.notOk(entriesAfterV2.some(e => e.startsWith('which@1.0.0-')),
'old @scope/which@1.0.0 store entry is removed after update')
})

tap.test('orphaned scoped store entries leave no empty scope directory when last dep is removed', async t => {
// https://github.com/npm/cli/issues/9440 — when the last package under a scope is orphaned, the now-empty @scope directory should also be pruned.
const graph = {
registry: [
{ name: '@scope/which', version: '1.0.0', dependencies: { isexe: '^1.0.0' } },
{ name: 'isexe', version: '1.0.0' },
],
root: {
name: 'myproject',
version: '1.0.0',
dependencies: { '@scope/which': '1.0.0' },
},
}
const { dir, registry } = await getRepo(graph)
const cache = fs.mkdtempSync(`${getTempDir()}/test-`)
const storeDir = path.join(dir, 'node_modules', '.store')

const arb1 = new Arborist({ path: dir, registry, packumentCache: new Map(), cache })
await arb1.reify({ installStrategy: 'linked' })

t.ok(fs.existsSync(path.join(storeDir, '@scope')), 'store has @scope directory after install')

// Remove the dependency
const pkgPath = path.join(dir, 'package.json')
const pkg = JSON.parse(fs.readFileSync(pkgPath, 'utf8'))
delete pkg.dependencies
fs.writeFileSync(pkgPath, JSON.stringify(pkg))

const arb2 = new Arborist({ path: dir, registry, packumentCache: new Map(), cache })
await arb2.reify({ installStrategy: 'linked' })

t.notOk(fs.existsSync(path.join(storeDir, '@scope')),
'empty @scope directory is pruned after the last scoped dep is removed')
})

tap.test('orphaned store entries are cleaned up on dependency removal', async t => {
const graph = {
registry: [
Expand Down
Loading