Skip to content

Commit 390ebfa

Browse files
fix(arborist): symlink workspace file: deps on non-workspace local packages (#9593)
Backport of #9591 to `release/v11`. Co-authored-by: Manzoor Wani <manzoorwani.jk@gmail.com>
1 parent a04cd84 commit 390ebfa

2 files changed

Lines changed: 98 additions & 1 deletion

File tree

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

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -266,7 +266,9 @@ module.exports = cls => class IsolatedReifier extends cls {
266266
}
267267

268268
// 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.
269-
const isLocal = (n) => n.isWorkspace || node.fsChildren?.has(n)
269+
// A file: dep surfaces as a Link edge whose resolved spec starts with file:; detect it from the edge so the target is treated as local even when it is absent from idealTree.fsChildren (a workspace consumer, or a target outside the repo root via npm link).
270+
const fileLinkTargets = new Set(edges.filter(e => e.to?.isLink && e.to.resolved?.startsWith('file:')).map(e => e.to.target))
271+
const isLocal = (n) => n.isWorkspace || node.fsChildren?.has(n) || fileLinkTargets.has(n)
270272
const optionalDeps = edges.filter(edge => edge.optional).map(edge => edge.to.target)
271273

272274
// Optional peers declared only in peerDependenciesMeta (e.g. `@types/react`) have no edge, so the materialization above misses them.

‎workspaces/arborist/test/isolated-mode.js‎

Lines changed: 95 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1891,6 +1891,101 @@ tap.test('npm link (external file: dep) with linked strategy', async t => {
18911891
t.notOk(storeEntries.some(e => e.startsWith('external-pkg@')), 'external-pkg is NOT in the store')
18921892
})
18931893

1894+
tap.test('workspace file: dependency on a non-workspace local package with linked strategy', async t => {
1895+
// Regression test for https://github.com/npm/cli/issues/9589
1896+
// A workspace declaring a file: dep on a local package that is NOT itself a workspace was silently skipped: no symlink, no error.
1897+
const graph = {
1898+
registry: [],
1899+
root: {
1900+
name: 'mono',
1901+
version: '1.0.0',
1902+
},
1903+
workspaces: [
1904+
{ name: 'ws-a', version: '1.0.0', dependencies: { 'local-dep': 'file:../../local-dep' } },
1905+
],
1906+
}
1907+
1908+
const { dir, registry } = await getRepo(graph)
1909+
1910+
// Create the non-workspace local package on disk, outside the workspaces globs
1911+
const depDir = path.join(dir, 'local-dep')
1912+
fs.mkdirSync(depDir, { recursive: true })
1913+
fs.writeFileSync(path.join(depDir, 'package.json'), JSON.stringify({
1914+
name: 'local-dep',
1915+
version: '1.0.0',
1916+
}))
1917+
fs.writeFileSync(path.join(depDir, 'index.js'), "module.exports = 'local-dep'")
1918+
1919+
const cache = fs.mkdtempSync(`${getTempDir()}/test-`)
1920+
const arborist = new Arborist({ path: dir, registry, packumentCache: new Map(), cache })
1921+
await arborist.reify({ installStrategy: 'linked' })
1922+
1923+
// The file dep should be symlinked into the workspace's node_modules
1924+
const linkPath = path.join(dir, 'packages', 'ws-a', 'node_modules', 'local-dep')
1925+
const stat = fs.lstatSync(linkPath)
1926+
t.ok(stat.isSymbolicLink(), 'local-dep is a symlink in the workspace node_modules')
1927+
1928+
// The symlink should resolve to the actual local directory
1929+
t.equal(fs.realpathSync(linkPath), fs.realpathSync(depDir), 'symlink points to the correct local directory')
1930+
1931+
// It must be symlinked directly, not extracted into the store
1932+
const storePath = path.join(dir, 'node_modules', '.store')
1933+
if (fs.existsSync(storePath)) {
1934+
t.notOk(fs.readdirSync(storePath).some(e => e.startsWith('local-dep@')), 'local-dep is NOT in the store')
1935+
}
1936+
1937+
// The package should be requireable from inside the workspace
1938+
t.ok(setupRequire(path.join(dir, 'packages', 'ws-a'))('local-dep'), 'local-dep can be required from the workspace')
1939+
})
1940+
1941+
tap.test('workspace file: dependency on a package outside the repo root with linked strategy', async t => {
1942+
// Regression test for the out-of-repo variant of https://github.com/npm/cli/issues/9589 (the real `npm --workspace link <external>` case, https://github.com/npm/cli/issues/9115).
1943+
// A workspace file: dep whose target resolves OUTSIDE the repo root was silently skipped.
1944+
// The target is not in idealTree.fsChildren, so the fix must detect it from the file: link edge.
1945+
const graph = {
1946+
registry: [],
1947+
root: {
1948+
name: 'mono',
1949+
version: '1.0.0',
1950+
},
1951+
workspaces: [
1952+
{ name: 'ws-a', version: '1.0.0', dependencies: { 'ext-pkg': 'file:../../../ext-pkg' } },
1953+
],
1954+
}
1955+
1956+
const { dir, registry } = await getRepo(graph)
1957+
1958+
// Create the external package OUTSIDE the repo root
1959+
const extDir = path.join(path.dirname(dir), 'ext-pkg')
1960+
fs.mkdirSync(extDir, { recursive: true })
1961+
fs.writeFileSync(path.join(extDir, 'package.json'), JSON.stringify({
1962+
name: 'ext-pkg',
1963+
version: '1.0.0',
1964+
}))
1965+
fs.writeFileSync(path.join(extDir, 'index.js'), "module.exports = 'ext-pkg'")
1966+
1967+
const cache = fs.mkdtempSync(`${getTempDir()}/test-`)
1968+
const arborist = new Arborist({ path: dir, registry, packumentCache: new Map(), cache })
1969+
await arborist.reify({ installStrategy: 'linked' })
1970+
1971+
// The file dep should be symlinked into the workspace's node_modules
1972+
const linkPath = path.join(dir, 'packages', 'ws-a', 'node_modules', 'ext-pkg')
1973+
const stat = fs.lstatSync(linkPath)
1974+
t.ok(stat.isSymbolicLink(), 'ext-pkg is a symlink in the workspace node_modules')
1975+
1976+
// The symlink should resolve to the actual external directory
1977+
t.equal(fs.realpathSync(linkPath), fs.realpathSync(extDir), 'symlink points to the correct external directory')
1978+
1979+
// It must be symlinked directly, not extracted into the store
1980+
const storePath = path.join(dir, 'node_modules', '.store')
1981+
if (fs.existsSync(storePath)) {
1982+
t.notOk(fs.readdirSync(storePath).some(e => e.startsWith('ext-pkg@')), 'ext-pkg is NOT in the store')
1983+
}
1984+
1985+
// The package should be requireable from inside the workspace
1986+
t.ok(setupRequire(path.join(dir, 'packages', 'ws-a'))('ext-pkg'), 'ext-pkg can be required from the workspace')
1987+
})
1988+
18941989
tap.test('subsequent linked install is a no-op', async t => {
18951990
const graph = {
18961991
registry: [

0 commit comments

Comments
 (0)