From 70b106be92ae7c09672cb17aa0c8f43b8c9895ee Mon Sep 17 00:00:00 2001 From: Ruy Adorno Date: Fri, 14 May 2021 16:37:47 -0400 Subject: [PATCH] feat: add ls workspaces - Add listing workspaces deps by default in `npm ls` - Add ability to filter the result tree by workspace using the -w config - Added tests and docs Fixes: https://github.com/npm/statusboard/issues/302 PR-URL: https://github.com/npm/cli/pull/3250 Credit: @ruyadorno Close: #3250 Reviewed-by: @isaacs --- docs/content/commands/npm-ls.md | 32 +++++ lib/ls.js | 34 ++++- .../test/lib/load-all-commands.js.test.cjs | 4 + tap-snapshots/test/lib/ls.js.test.cjs | 59 ++++++++- .../test/lib/utils/npm-usage.js.test.cjs | 4 + test/lib/ls.js | 124 ++++++++++++++++-- 6 files changed, 236 insertions(+), 21 deletions(-) diff --git a/docs/content/commands/npm-ls.md b/docs/content/commands/npm-ls.md index 505dfd9c4316c..3c662176327bf 100644 --- a/docs/content/commands/npm-ls.md +++ b/docs/content/commands/npm-ls.md @@ -177,6 +177,38 @@ When used with `npm ls`, only show packages that are linked. When set to true, npm uses unicode characters in the tree output. When false, it uses ascii characters instead of unicode glyphs. +#### `workspace` + +* Default: +* Type: String (can be set multiple times) + +Enable running a command in the context of the configured workspaces of the +current project while filtering by running only the workspaces defined by +this configuration option. + +Valid values for the `workspace` config are either: + +* Workspace names +* Path to a workspace directory +* Path to a parent workspace directory (will result to selecting all of the + nested workspaces) + +When set for the `npm init` command, this may be set to the folder of a +workspace which does not yet exist, to create the folder and set it up as a +brand new workspace within the project. + +This value is not exported to the environment for child processes. + +#### `workspaces` + +* Default: false +* Type: Boolean + +Enable running a command in the context of **all** the configured +workspaces. + +This value is not exported to the environment for child processes. + ### See Also diff --git a/lib/ls.js b/lib/ls.js index 569156ea2a730..4e504912a0175 100644 --- a/lib/ls.js +++ b/lib/ls.js @@ -20,9 +20,9 @@ const _parent = Symbol('parent') const _problems = Symbol('problems') const _required = Symbol('required') const _type = Symbol('type') -const BaseCommand = require('./base-command.js') +const ArboristWorkspaceCmd = require('./workspaces/arborist-cmd.js') -class LS extends BaseCommand { +class LS extends ArboristWorkspaceCmd { /* istanbul ignore next - see test/lib/load-all-commands.js */ static get description () { return 'List installed packages' @@ -50,6 +50,7 @@ class LS extends BaseCommand { 'omit', 'link', 'unicode', + ...super.params, ] } @@ -88,6 +89,25 @@ class LS extends BaseCommand { }) const tree = await this.initTree({arb, args }) + // filters by workspaces nodes when using -w + // We only have to filter the first layer of edges, so we don't + // explore anything that isn't part of the selected workspace set. + let wsNodes + if (this.workspaces && this.workspaces.length) + wsNodes = arb.workspaceNodes(tree, this.workspaces) + const filterBySelectedWorkspaces = edge => { + if (!wsNodes || !wsNodes.length) + return true + + if (edge.from.isProjectRoot) { + return edge.to && + edge.to.isWorkspace & + wsNodes.includes(edge.to.target) + } + + return true + } + const seenItems = new Set() const seenNodes = new Map() const problems = new Set() @@ -109,11 +129,14 @@ class LS extends BaseCommand { // `nodeResult` is going to be the returned `item` from `visit` getChildren (node, nodeResult) { const seenPaths = new Set() + const workspace = node.isWorkspace + const currentDepth = workspace ? 0 : node[_depth] const shouldSkipChildren = - !(node instanceof Arborist.Node) || (node[_depth] > depthToPrint) + !(node instanceof Arborist.Node) || (currentDepth > depthToPrint) return (shouldSkipChildren) ? [] : [...(node.target || node).edgesOut.values()] + .filter(filterBySelectedWorkspaces) .filter(filterByEdgesTypes({ dev, development, @@ -129,7 +152,7 @@ class LS extends BaseCommand { .sort(sortAlphabetically) .map(augmentNodesWithMetadata({ args, - currentDepth: node[_depth], + currentDepth, nodeResult, seenNodes, })) @@ -257,7 +280,8 @@ const augmentItemWithIncludeMetadata = (node, item) => { const getHumanOutputItem = (node, { args, color, global, long }) => { const { pkgid, path } = node - let printable = pkgid + const workspacePkgId = color ? chalk.green(pkgid) : pkgid + let printable = node.isWorkspace ? workspacePkgId : pkgid // special formatting for top-level package name if (node.isRoot) { diff --git a/tap-snapshots/test/lib/load-all-commands.js.test.cjs b/tap-snapshots/test/lib/load-all-commands.js.test.cjs index c549c057c3761..f2d40d4161494 100644 --- a/tap-snapshots/test/lib/load-all-commands.js.test.cjs +++ b/tap-snapshots/test/lib/load-all-commands.js.test.cjs @@ -539,6 +539,8 @@ Options: [-a|--all] [--json] [-l|--long] [-p|--parseable] [-g|--global] [--depth ] [--omit [--omit ...]] [--link] [--unicode] +[-w|--workspace [-w|--workspace ...]] +[-ws|--workspaces] alias: la @@ -587,6 +589,8 @@ Options: [-a|--all] [--json] [-l|--long] [-p|--parseable] [-g|--global] [--depth ] [--omit [--omit ...]] [--link] [--unicode] +[-w|--workspace [-w|--workspace ...]] +[-ws|--workspaces] alias: list diff --git a/tap-snapshots/test/lib/ls.js.test.cjs b/tap-snapshots/test/lib/ls.js.test.cjs index 04f0be31f5803..d5d23451b555f 100644 --- a/tap-snapshots/test/lib/ls.js.test.cjs +++ b/tap-snapshots/test/lib/ls.js.test.cjs @@ -478,17 +478,64 @@ exports[`test/lib/ls.js TAP ls json read problems > should print empty result 1` ` +exports[`test/lib/ls.js TAP ls loading a tree containing workspaces > should filter by parent folder workspace config 1`] = ` +workspaces-tree@1.0.0 {CWD}/tap-testdir-ls-ls-loading-a-tree-containing-workspaces ++-- e@1.0.0 -> {CWD}/tap-testdir-ls-ls-loading-a-tree-containing-workspaces/group/e +\`-- f@1.0.0 -> {CWD}/tap-testdir-ls-ls-loading-a-tree-containing-workspaces/group/f + +` + exports[`test/lib/ls.js TAP ls loading a tree containing workspaces > should filter single workspace 1`] = ` -filter-by-child-of-missing-dep@1.0.0 {CWD}/tap-testdir-ls-ls-loading-a-tree-containing-workspaces -\`-- a@1.0.0 -> {CWD}/tap-testdir-ls-ls-loading-a-tree-containing-workspaces/a +workspaces-tree@1.0.0 {CWD}/tap-testdir-ls-ls-loading-a-tree-containing-workspaces ++-- a@1.0.0 -> {CWD}/tap-testdir-ls-ls-loading-a-tree-containing-workspaces/a +| \`-- d@1.0.0 deduped -> {CWD}/tap-testdir-ls-ls-loading-a-tree-containing-workspaces/d +\`-- d@1.0.0 -> {CWD}/tap-testdir-ls-ls-loading-a-tree-containing-workspaces/d ` -exports[`test/lib/ls.js TAP ls loading a tree containing workspaces > should list workspaces properly 1`] = ` -filter-by-child-of-missing-dep@1.0.0 {CWD}/tap-testdir-ls-ls-loading-a-tree-containing-workspaces +exports[`test/lib/ls.js TAP ls loading a tree containing workspaces > should filter using workspace config 1`] = ` +workspaces-tree@1.0.0 {CWD}/tap-testdir-ls-ls-loading-a-tree-containing-workspaces +-- a@1.0.0 -> {CWD}/tap-testdir-ls-ls-loading-a-tree-containing-workspaces/a -| \`-- c@1.0.0 -\`-- b@1.0.0 -> {CWD}/tap-testdir-ls-ls-loading-a-tree-containing-workspaces/b +| +-- c@1.0.0 +| \`-- d@1.0.0 deduped -> {CWD}/tap-testdir-ls-ls-loading-a-tree-containing-workspaces/d +\`-- d@1.0.0 -> {CWD}/tap-testdir-ls-ls-loading-a-tree-containing-workspaces/d + \`-- foo@1.1.1 + \`-- bar@1.0.0 + +` + +exports[`test/lib/ls.js TAP ls loading a tree containing workspaces > should list --all workspaces properly 1`] = ` +workspaces-tree@1.0.0 {CWD}/tap-testdir-ls-ls-loading-a-tree-containing-workspaces ++-- a@1.0.0 -> {CWD}/tap-testdir-ls-ls-loading-a-tree-containing-workspaces/a +| +-- c@1.0.0 +| \`-- d@1.0.0 deduped -> {CWD}/tap-testdir-ls-ls-loading-a-tree-containing-workspaces/d ++-- b@1.0.0 -> {CWD}/tap-testdir-ls-ls-loading-a-tree-containing-workspaces/b ++-- d@1.0.0 -> {CWD}/tap-testdir-ls-ls-loading-a-tree-containing-workspaces/d +| \`-- foo@1.1.1 +| \`-- bar@1.0.0 ++-- e@1.0.0 -> {CWD}/tap-testdir-ls-ls-loading-a-tree-containing-workspaces/group/e +\`-- f@1.0.0 -> {CWD}/tap-testdir-ls-ls-loading-a-tree-containing-workspaces/group/f + +` + +exports[`test/lib/ls.js TAP ls loading a tree containing workspaces > should list workspaces properly with default configs 1`] = ` +workspaces-tree@1.0.0 {CWD}/tap-testdir-ls-ls-loading-a-tree-containing-workspaces ++-- a@1.0.0 -> {CWD}/tap-testdir-ls-ls-loading-a-tree-containing-workspaces/a +| +-- c@1.0.0 +| \`-- d@1.0.0 deduped -> {CWD}/tap-testdir-ls-ls-loading-a-tree-containing-workspaces/d ++-- b@1.0.0 -> {CWD}/tap-testdir-ls-ls-loading-a-tree-containing-workspaces/b ++-- d@1.0.0 -> {CWD}/tap-testdir-ls-ls-loading-a-tree-containing-workspaces/d +| \`-- foo@1.1.1 ++-- e@1.0.0 -> {CWD}/tap-testdir-ls-ls-loading-a-tree-containing-workspaces/group/e +\`-- f@1.0.0 -> {CWD}/tap-testdir-ls-ls-loading-a-tree-containing-workspaces/group/f + +` + +exports[`test/lib/ls.js TAP ls loading a tree containing workspaces > should print all tree and filter by dep within only the ws subtree 1`] = ` +workspaces-tree@1.0.0 {CWD}/tap-testdir-ls-ls-loading-a-tree-containing-workspaces +\`-- d@1.0.0 -> {CWD}/tap-testdir-ls-ls-loading-a-tree-containing-workspaces/d + \`-- foo@1.1.1 + \`-- bar@1.0.0 ` diff --git a/tap-snapshots/test/lib/utils/npm-usage.js.test.cjs b/tap-snapshots/test/lib/utils/npm-usage.js.test.cjs index a67819eb05bdc..a81a8f44b30b9 100644 --- a/tap-snapshots/test/lib/utils/npm-usage.js.test.cjs +++ b/tap-snapshots/test/lib/utils/npm-usage.js.test.cjs @@ -640,6 +640,8 @@ All commands: [-a|--all] [--json] [-l|--long] [-p|--parseable] [-g|--global] [--depth ] [--omit [--omit ...]] [--link] [--unicode] + [-w|--workspace [-w|--workspace ...]] + [-ws|--workspaces] alias: la @@ -682,6 +684,8 @@ All commands: [-a|--all] [--json] [-l|--long] [-p|--parseable] [-g|--global] [--depth ] [--omit [--omit ...]] [--link] [--unicode] + [-w|--workspace [-w|--workspace ...]] + [-ws|--workspaces] alias: list diff --git a/test/lib/ls.js b/test/lib/ls.js index 276a06180f654..2cde319463a9e 100644 --- a/test/lib/ls.js +++ b/test/lib/ls.js @@ -1407,14 +1407,16 @@ t.test('ls', (t) => { }) }) - t.test('loading a tree containing workspaces', (t) => { - npm.prefix = t.testdir({ + t.test('loading a tree containing workspaces', async (t) => { + npm.localPrefix = npm.prefix = t.testdir({ 'package.json': JSON.stringify({ - name: 'filter-by-child-of-missing-dep', + name: 'workspaces-tree', version: '1.0.0', workspaces: [ './a', './b', + './d', + './group/*', ], }), node_modules: { @@ -1426,6 +1428,21 @@ t.test('ls', (t) => { version: '1.0.0', }), }, + d: t.fixture('symlink', '../d'), + e: t.fixture('symlink', '../group/e'), + f: t.fixture('symlink', '../group/f'), + foo: { + 'package.json': JSON.stringify({ + name: 'foo', + version: '1.1.1', + dependencies: { + bar: '^1.0.0', + }, + }), + }, + bar: { + 'package.json': JSON.stringify({ name: 'bar', version: '1.0.0' }), + }, }, a: { 'package.json': JSON.stringify({ @@ -1433,6 +1450,7 @@ t.test('ls', (t) => { version: '1.0.0', dependencies: { c: '^1.0.0', + d: '^1.0.0', }, }), }, @@ -1442,18 +1460,104 @@ t.test('ls', (t) => { version: '1.0.0', }), }, + d: { + 'package.json': JSON.stringify({ + name: 'd', + version: '1.0.0', + dependencies: { + foo: '^1.1.1', + }, + }), + }, + group: { + e: { + 'package.json': JSON.stringify({ + name: 'e', + version: '1.0.0', + }), + }, + f: { + 'package.json': JSON.stringify({ + name: 'f', + version: '1.0.0', + }), + }, + }, }) - ls.exec([], (err) => { - t.error(err, 'should NOT have ELSPROBLEMS error code') - t.matchSnapshot(redactCwd(result), 'should list workspaces properly') + await new Promise((res, rej) => { + config.all = false + config.depth = 0 + npm.color = true + ls.exec([], (err) => { + if (err) + rej(err) + + t.matchSnapshot(redactCwd(result), + 'should list workspaces properly with default configs') + config.all = true + config.depth = Infinity + npm.color = false + res() + }) + }) + + // --all + await new Promise((res, rej) => { + ls.exec([], (err) => { + if (err) + rej(err) + + t.matchSnapshot(redactCwd(result), + 'should list --all workspaces properly') + res() + }) + }) + + // filter out a single workspace using args + await new Promise((res, rej) => { + ls.exec(['d'], (err) => { + if (err) + rej(err) - // should also be able to filter out one of the workspaces - ls.exec(['a'], (err) => { - t.error(err, 'should NOT have ELSPROBLEMS error code when filter') t.matchSnapshot(redactCwd(result), 'should filter single workspace') + res() + }) + }) + + // filter out a single workspace and its deps using workspaces filters + await new Promise((res, rej) => { + ls.execWorkspaces([], ['a'], (err) => { + if (err) + rej(err) + + t.matchSnapshot(redactCwd(result), + 'should filter using workspace config') + res() + }) + }) + + // filter out a workspace by parent path + await new Promise((res, rej) => { + ls.execWorkspaces([], ['./group'], (err) => { + if (err) + rej(err) + + t.matchSnapshot(redactCwd(result), + 'should filter by parent folder workspace config') + res() + }) + }) + + // filter by a dep within a workspaces sub tree + await new Promise((res, rej) => { + ls.execWorkspaces(['bar'], ['d'], (err) => { + if (err) + rej(err) - t.end() + t.matchSnapshot(redactCwd(result), + 'should print all tree and filter by dep within only the ws subtree') + res() }) }) })