Skip to content

Commit a105799

Browse files
manzoorwanijkowlstronaut
authored andcommitted
fix(arborist): link meta-only optional peers in linked strategy
1 parent 275bc69 commit a105799

2 files changed

Lines changed: 66 additions & 0 deletions

File tree

workspaces/arborist/lib/arborist/isolated-reifier.js

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -214,6 +214,22 @@ module.exports = cls => class IsolatedReifier extends cls {
214214
// 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.
215215
const isLocal = (n) => n.isWorkspace || node.fsChildren?.has(n)
216216
const optionalDeps = edges.filter(edge => edge.optional).map(edge => edge.to.target)
217+
218+
// Optional peers declared only in peerDependenciesMeta (e.g. `@types/react`) have no edge, so the materialization above misses them.
219+
// Resolve each from the tree and link it; if nobody provides it, node.resolve finds nothing and it stays omitted.
220+
const peerMeta = node.package.peerDependenciesMeta
221+
if (peerMeta) {
222+
const resolvedNames = new Set([...nonOptionalDeps, ...optionalDeps].map(n => n.name))
223+
for (const peerName in peerMeta) {
224+
if (!peerMeta[peerName]?.optional || resolvedNames.has(peerName)) {
225+
continue
226+
}
227+
const resolved = node.resolve(peerName)?.target
228+
if (resolved && resolved !== node && !resolved.inert && !isLocal(resolved)) {
229+
optionalDeps.push(resolved)
230+
}
231+
}
232+
}
217233
result.localDependencies = await Promise.all(nonOptionalDeps.filter(isLocal).map(n => this.#workspaceProxy(n)))
218234
result.externalDependencies = await Promise.all(nonOptionalDeps.filter(n => !isLocal(n) && !n.inert).map(n => this.#externalProxy(n)))
219235
result.externalOptionalDependencies = await Promise.all(optionalDeps.filter(n => !n.inert).map(n => this.#externalProxy(n)))

workspaces/arborist/test/isolated-mode.js

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1310,6 +1310,56 @@ tap.test('failing optional peer deps are not installed', async t => {
13101310
t.notOk(setupRequire(dir)('bar', 'which'), 'Failing optional peer deps should not be installed')
13111311
})
13121312

1313+
tap.test('optional peer declared only in peerDependenciesMeta is materialized when provided', async t => {
1314+
// Regression for npm/cli#9460.
1315+
// `bar` declares `which` as an optional peer via peerDependenciesMeta only, with no peerDependencies entry, so no edge is created for it.
1316+
// The workspace provides `which`, so under the linked strategy `which` should be linked into `bar`'s store node_modules (matching pnpm).
1317+
// `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.
1318+
const graph = {
1319+
registry: [
1320+
{ name: 'which', version: '1.0.0' },
1321+
{ name: 'bar', version: '1.0.0', peerDependenciesMeta: { which: { optional: true } } },
1322+
],
1323+
root: { name: 'foo', version: '1.2.3' },
1324+
workspaces: [
1325+
{ name: 'app', version: '1.0.0', dependencies: { bar: '*', which: '1.0.0' } },
1326+
],
1327+
}
1328+
1329+
const { dir, registry } = await getRepo(graph)
1330+
1331+
// Note that we override this cache to prevent interference from other tests
1332+
const cache = fs.mkdtempSync(`${getTempDir()}/test-`)
1333+
const arborist = new Arborist({ path: dir, registry, packumentCache: new Map(), cache })
1334+
await arborist.reify({ installStrategy: 'linked' })
1335+
1336+
t.ok(setupRequire(path.join(dir, 'packages', 'app'))('bar', 'which'),
1337+
'optional peer provided by the workspace is materialized into bar store node_modules')
1338+
})
1339+
1340+
tap.test('optional peer declared only in peerDependenciesMeta is omitted when not provided', async t => {
1341+
// Counterpart to the regression above: when nobody provides the optional peer it must stay omitted, preserving "optional" semantics.
1342+
const graph = {
1343+
registry: [
1344+
{ name: 'bar', version: '1.0.0', peerDependenciesMeta: { which: { optional: true } } },
1345+
],
1346+
root: { name: 'foo', version: '1.2.3' },
1347+
workspaces: [
1348+
{ name: 'app', version: '1.0.0', dependencies: { bar: '*' } },
1349+
],
1350+
}
1351+
1352+
const { dir, registry } = await getRepo(graph)
1353+
1354+
// Note that we override this cache to prevent interference from other tests
1355+
const cache = fs.mkdtempSync(`${getTempDir()}/test-`)
1356+
const arborist = new Arborist({ path: dir, registry, packumentCache: new Map(), cache })
1357+
await arborist.reify({ installStrategy: 'linked' })
1358+
1359+
t.notOk(setupRequire(path.join(dir, 'packages', 'app'))('bar', 'which'),
1360+
'optional peer that nobody provides is not materialized')
1361+
})
1362+
13131363
// Virtual packages are 2 packages that have the same version but are
13141364
// duplicated on disk to solve peer-dependency conflict.
13151365
tap.test('virtual packages', async t => {

0 commit comments

Comments
 (0)