diff --git a/workspaces/arborist/lib/arborist/load-actual.js b/workspaces/arborist/lib/arborist/load-actual.js index d6615caa42fb2..cfdca0aa3d538 100644 --- a/workspaces/arborist/lib/arborist/load-actual.js +++ b/workspaces/arborist/lib/arborist/load-actual.js @@ -93,6 +93,8 @@ module.exports = cls => class ActualLoader extends cls { transplantFilter = () => true, ignoreMissing = false, forceActual = false, + // always present: the public loadActual merges this.options, which sets installStrategy + installStrategy, } = options this.#filter = filter this.#transplantFilter = transplantFilter @@ -173,6 +175,20 @@ module.exports = cls => class ActualLoader extends cls { } } await Promise.all(promises) + + // Linked undeclared workspaces aren't symlinked into root node_modules, so their edges resolve to null and flags never propagate. + // Synthesize the links from the loaded targets. Gated to linked; under hoisted a null workspace edge is a real missing link. + if (installStrategy === 'linked') { + for (const [name, path] of this.#actualTree.workspaces.entries()) { + const edge = this.#actualTree.edgesOut.get(name) + // skip workspaces already linked into root node_modules (declared deps) + if (edge.to) { + continue + } + const target = this.#cache.get(path) + new Link({ parent: this.#actualTree, name, realpath: path, target, pkg: target.package }) + } + } } // .npm-extension runs before packageExtensions, matching the ideal-tree resolution order diff --git a/workspaces/arborist/lib/calc-dep-flags.js b/workspaces/arborist/lib/calc-dep-flags.js index 5f2484858094d..965127f5ac9dd 100644 --- a/workspaces/arborist/lib/calc-dep-flags.js +++ b/workspaces/arborist/lib/calc-dep-flags.js @@ -35,12 +35,32 @@ const calcDepFlags = (tree, resetRoot = true) => { if (node.target == null) { continue } - node.target.dev = node.dev - node.target.optional = node.optional - node.target.devOptional = node.devOptional - node.target.peer = node.peer - node.target.extraneous = node.extraneous - queue.push(node.target) + // Only unset flags, matching the edge walk below, so multiple links to one target can't clobber a correct value. + let changed = false + if (node.target.dev && !node.dev) { + node.target.dev = false + changed = true + } + if (node.target.optional && !node.optional) { + node.target.optional = false + changed = true + } + if (node.target.devOptional && !node.devOptional) { + node.target.devOptional = false + changed = true + } + if (node.target.peer && !node.peer) { + node.target.peer = false + changed = true + } + if (node.target.extraneous && !node.extraneous) { + node.target.extraneous = false + changed = true + } + // queue target on first visit so its deps are walked + if (changed || !seen.has(node.target)) { + queue.push(node.target) + } continue } diff --git a/workspaces/arborist/test/arborist/load-actual.js b/workspaces/arborist/test/arborist/load-actual.js index 9d724e934fdca..d21a9421d86b9 100644 --- a/workspaces/arborist/test/arborist/load-actual.js +++ b/workspaces/arborist/test/arborist/load-actual.js @@ -303,6 +303,49 @@ t.test('transplant workspace targets, even if links not present', async t => { }), 'do not transplant node named "a"') }) +t.test('linked strategy propagates dep flags into undeclared workspaces', async t => { + // Undeclared workspaces aren't in root node_modules under linked; without the fix their edges resolve to null and stay dev. + // `tools` is also a devDependency of `app`, so it is reached by both a prod root link and a dev `app` link. + // `app` is linked into root node_modules, so its edge already resolves and the synthesis is skipped. + const path = t.testdir({ + 'package.json': JSON.stringify({ + name: 'root', + workspaces: ['packages/*'], + }), + node_modules: { + '@test': { + app: t.fixture('symlink', '../../packages/app'), + }, + }, + packages: { + app: { + 'package.json': JSON.stringify({ + name: '@test/app', + version: '1.0.0', + devDependencies: { '@test/tools': '*' }, + }), + node_modules: { + '@test': { + tools: t.fixture('symlink', '../../../tools'), + }, + }, + }, + tools: { + 'package.json': JSON.stringify({ + name: '@test/tools', + version: '1.0.0', + }), + }, + }, + }) + const tree = await loadActual(path, { installStrategy: 'linked' }) + const byLocation = loc => [...tree.inventory.values()].find(n => n.location === loc) + const app = byLocation('packages/app') + const tools = byLocation('packages/tools') + t.equal(app.dev, false, 'app workspace is prod') + t.equal(tools.dev, false, 'tools workspace is prod, not overwritten by the dev app link') +}) + t.test('load workspaces when loading from hidden lockfile', async t => { const path = t.testdir({ 'package.json': JSON.stringify({ diff --git a/workspaces/arborist/test/calc-dep-flags.js b/workspaces/arborist/test/calc-dep-flags.js index b371ae55f6e4b..70a5ec48b6a46 100644 --- a/workspaces/arborist/test/calc-dep-flags.js +++ b/workspaces/arborist/test/calc-dep-flags.js @@ -343,6 +343,37 @@ t.test('set parents to not extraneous when visiting', t => { t.end() }) +t.test('multiple links to one target keep the most permissive flags', async t => { + // `tools` is reached by a prod root link and a dev `app` link; the dev link must not overwrite the prod dev flag. + const root = new Node({ + path: '/r', + realpath: '/r', + pkg: { name: 'root', workspaces: ['app', 'tools'] }, + }) + const app = new Node({ + path: '/r/app', + realpath: '/r/app', + root, + pkg: { name: 'app', version: '1.0.0', devDependencies: { tools: '*' } }, + }) + const tools = new Node({ + path: '/r/tools', + realpath: '/r/tools', + root, + pkg: { name: 'tools', version: '1.0.0' }, + }) + new Link({ name: 'app', parent: root, realpath: '/r/app', target: app }) + new Link({ name: 'tools', parent: root, realpath: '/r/tools', target: tools }) + new Link({ name: 'tools', parent: app, realpath: '/r/tools', target: tools }) + root.workspaces = new Map([['app', '/r/app'], ['tools', '/r/tools']]) + + calcDepFlags(root) + + t.equal(tools.dev, false, 'tools is prod via the root workspace link') + t.equal(app.dev, false, 'app is prod via the root workspace link') + t.end() +}) + t.test('check null target in link', async t => { const root = new Link({ path: '/some/path',