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/load-actual.js
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,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
Expand Down Expand Up @@ -171,6 +173,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 })
}
}
}

if (!ignoreMissing) {
Expand Down
32 changes: 26 additions & 6 deletions workspaces/arborist/lib/calc-dep-flags.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down
43 changes: 43 additions & 0 deletions workspaces/arborist/test/arborist/load-actual.js
Original file line number Diff line number Diff line change
Expand Up @@ -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({
Expand Down
31 changes: 31 additions & 0 deletions workspaces/arborist/test/calc-dep-flags.js
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down
Loading