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
16 changes: 16 additions & 0 deletions workspaces/arborist/lib/arborist/isolated-reifier.js
Original file line number Diff line number Diff line change
Expand Up @@ -259,6 +259,22 @@ module.exports = cls => class IsolatedReifier extends cls {
// local `file:` deps (non-workspace fsChildren) should be treated as local dependencies, not external, so they get symlinked directly instead of being extracted into the store.
const isLocal = (n) => n.isWorkspace || node.fsChildren?.has(n)
const optionalDeps = edges.filter(edge => edge.optional).map(edge => edge.to.target)

// Optional peers declared only in peerDependenciesMeta (e.g. `@types/react`) have no edge, so the materialization above misses them.
// Resolve each from the tree and link it; if nobody provides it, node.resolve finds nothing and it stays omitted.
const peerMeta = node.package.peerDependenciesMeta
if (peerMeta) {
const resolvedNames = new Set([...nonOptionalDeps, ...optionalDeps].map(n => n.name))
for (const peerName in peerMeta) {
if (!peerMeta[peerName]?.optional || resolvedNames.has(peerName)) {
continue
}
const resolved = node.resolve(peerName)?.target
if (resolved && resolved !== node && !resolved.inert && !isLocal(resolved)) {
optionalDeps.push(resolved)
}
}
}
result.localDependencies = await Promise.all(nonOptionalDeps.filter(isLocal).map(n => this.#workspaceProxy(n)))
result.externalDependencies = await Promise.all(nonOptionalDeps.filter(n => !isLocal(n) && !n.inert).map(n => this.#externalProxy(n)))
result.externalOptionalDependencies = await Promise.all(optionalDeps.filter(n => !n.inert).map(n => this.#externalProxy(n)))
Expand Down
50 changes: 50 additions & 0 deletions workspaces/arborist/test/isolated-mode.js
Original file line number Diff line number Diff line change
Expand Up @@ -1544,6 +1544,56 @@ tap.test('failing optional peer deps are not installed', async t => {
t.notOk(setupRequire(dir)('bar', 'which'), 'Failing optional peer deps should not be installed')
})

tap.test('optional peer declared only in peerDependenciesMeta is materialized when provided', async t => {
// Regression for npm/cli#9460.
// `bar` declares `which` as an optional peer via peerDependenciesMeta only, with no peerDependencies entry, so no edge is created for it.
// The workspace provides `which`, so under the linked strategy `which` should be linked into `bar`'s store node_modules (matching pnpm).
// `which` is not a root dependency, so it is not hoisted to the top-level node_modules where parent-dir lookup would mask the result.
const graph = {
registry: [
{ name: 'which', version: '1.0.0' },
{ name: 'bar', version: '1.0.0', peerDependenciesMeta: { which: { optional: true } } },
],
root: { name: 'foo', version: '1.2.3' },
workspaces: [
{ name: 'app', version: '1.0.0', dependencies: { bar: '*', which: '1.0.0' } },
],
}

const { dir, registry } = await getRepo(graph)

// Note that we override this cache to prevent interference from other tests
const cache = fs.mkdtempSync(`${getTempDir()}/test-`)
const arborist = new Arborist({ path: dir, registry, packumentCache: new Map(), cache })
await arborist.reify({ installStrategy: 'linked' })

t.ok(setupRequire(path.join(dir, 'packages', 'app'))('bar', 'which'),
'optional peer provided by the workspace is materialized into bar store node_modules')
})

tap.test('optional peer declared only in peerDependenciesMeta is omitted when not provided', async t => {
// Counterpart to the regression above: when nobody provides the optional peer it must stay omitted, preserving "optional" semantics.
const graph = {
registry: [
{ name: 'bar', version: '1.0.0', peerDependenciesMeta: { which: { optional: true } } },
],
root: { name: 'foo', version: '1.2.3' },
workspaces: [
{ name: 'app', version: '1.0.0', dependencies: { bar: '*' } },
],
}

const { dir, registry } = await getRepo(graph)

// Note that we override this cache to prevent interference from other tests
const cache = fs.mkdtempSync(`${getTempDir()}/test-`)
const arborist = new Arborist({ path: dir, registry, packumentCache: new Map(), cache })
await arborist.reify({ installStrategy: 'linked' })

t.notOk(setupRequire(path.join(dir, 'packages', 'app'))('bar', 'which'),
'optional peer that nobody provides is not materialized')
})

// Virtual packages are 2 packages that have the same version but are
// duplicated on disk to solve peer-dependency conflict.
tap.test('virtual packages', async t => {
Expand Down
Loading