diff --git a/workspaces/arborist/lib/arborist/reify.js b/workspaces/arborist/lib/arborist/reify.js index 0057d38f11756..22ea432ee48d6 100644 --- a/workspaces/arborist/lib/arborist/reify.js +++ b/workspaces/arborist/lib/arborist/reify.js @@ -483,17 +483,29 @@ module.exports = cls => class Reifier extends cls { process.emit('time', 'reify:trashOmits') - const filter = node => - node.top.isProjectRoot && - ( - node.peer && this[_omitPeer] || - node.dev && this[_omitDev] || - node.optional && this[_omitOptional] || - node.devOptional && this[_omitOptional] && this[_omitDev] - ) - - for (const node of this.idealTree.inventory.filter(filter)) { - this[_addNodeToTrashList](node) + for (const node of this.idealTree.inventory.values()) { + const { top } = node + + // if the top is not the root or workspace then we do not want to omit it + if (!top.isProjectRoot && !top.isWorkspace) { + continue + } + + // if a diff filter has been created, then we do not omit the node if the + // top node is not in that set + if (this.diff?.filterSet?.size && !this.diff.filterSet.has(top)) { + continue + } + + // omit node if the dep type matches any omit flags that were set + if ( + node.peer && this[_omitPeer] || + node.dev && this[_omitDev] || + node.optional && this[_omitOptional] || + node.devOptional && this[_omitOptional] && this[_omitDev] + ) { + this[_addNodeToTrashList](node) + } } process.emit('timeEnd', 'reify:trashOmits') diff --git a/workspaces/arborist/test/arborist/reify.js b/workspaces/arborist/test/arborist/reify.js index e730b22b332a2..1f8a0d6310a4e 100644 --- a/workspaces/arborist/test/arborist/reify.js +++ b/workspaces/arborist/test/arborist/reify.js @@ -1187,6 +1187,55 @@ t.test('workspaces', t => { t.test('reify simple-workspaces', t => t.resolveMatchSnapshot(printReified(fixture(t, 'workspaces-simple')), 'should reify simple workspaces')) + t.test('reify workspaces omit dev dependencies', async t => { + const runCase = async (t, opts) => { + const path = fixture(t, 'workspaces-conflicting-dev-deps') + const rootAjv = resolve(path, 'node_modules/ajv/package.json') + const ajvOfPkgA = resolve(path, 'a/node_modules/ajv/package.json') + const ajvOfPkgB = resolve(path, 'b/node_modules/ajv/package.json') + + t.equal(fs.existsSync(rootAjv), true, 'root ajv exists') + t.equal(fs.existsSync(ajvOfPkgA), true, 'ajv under package a node_modules exists') + t.equal(fs.existsSync(ajvOfPkgB), true, 'ajv under package a node_modules exists') + + await reify(path, { omit: ['dev'], ...opts }) + + return { + root: { exists: () => fs.existsSync(rootAjv) }, + a: { exists: () => fs.existsSync(ajvOfPkgA) }, + b: { exists: () => fs.existsSync(ajvOfPkgB) }, + } + } + + await t.test('default', async t => { + const { root, a, b } = await runCase(t) + t.equal(root.exists(), false, 'root') + t.equal(a.exists(), false, 'a') + t.equal(b.exists(), false, 'b') + }) + + await t.test('workspaces only', async t => { + const { root, a, b } = await runCase(t, { workspaces: ['a'] }) + t.equal(root.exists(), false, 'root') + t.equal(a.exists(), false, 'a') + t.equal(b.exists(), true, 'b') + }) + + await t.test('workspaces + root', async t => { + const { root, a, b } = await runCase(t, { workspaces: ['a'], includeWorkspaceRoot: true }) + t.equal(root.exists(), false, 'root') + t.equal(a.exists(), false, 'a') + t.equal(b.exists(), true, 'b') + }) + + await t.test('disable workspaces', async t => { + const { root, a, b } = await runCase(t, { workspacesEnabled: false }) + t.equal(root.exists(), false, 'root') + t.equal(a.exists(), true, 'a') + t.equal(b.exists(), true, 'b') + }) + }) + t.test('reify workspaces lockfile', async t => { const path = fixture(t, 'workspaces-simple') await reify(path) diff --git a/workspaces/arborist/test/fixtures/reify-cases/workspaces-conflicting-dev-deps.js b/workspaces/arborist/test/fixtures/reify-cases/workspaces-conflicting-dev-deps.js new file mode 100644 index 0000000000000..1a6d4d04a970b --- /dev/null +++ b/workspaces/arborist/test/fixtures/reify-cases/workspaces-conflicting-dev-deps.js @@ -0,0 +1,58 @@ +// generated from test/fixtures/workspaces-simple +module.exports = t => { + const path = t.testdir({ + "node_modules": { + ajv: { + 'package.json': JSON.stringify({ + name: 'ajv', + version: '6.10.2', + }), + } + }, + "a": { + "package.json": JSON.stringify({ + "name": "a", + "version": "1.0.0", + "devDependencies": { + "ajv": "4.11.2" + } + }), + "node_modules": { + ajv: { + 'package.json': JSON.stringify({ + name: 'ajv', + version: '4.11.2', + }), + } + }, + }, + "b": { + "package.json": JSON.stringify({ + "name": "b", + "version": "1.0.0", + "devDependencies": { + "ajv": "5.11.2" + } + }), + "node_modules": { + ajv: { + 'package.json': JSON.stringify({ + name: 'ajv', + version: '5.11.2', + }), + } + }, + }, + "package.json": JSON.stringify({ + "name": "workspace-conflicting-dev-deps", + "devDependencies": { + "ajv": "6.10.2" + }, + "workspaces": [ + "a", + "b" + ] + }) +}) + return path +}