From 76c99c77ccc01e041afd56e71ff95d9e247bcb5f Mon Sep 17 00:00:00 2001 From: Ruy Adorno Date: Wed, 22 Jul 2020 17:49:17 -0400 Subject: fix: deduped+filter issues - When using args to filter out items deduped pkgs were broken - Fixed highlight color when using filter args - Changed default behavior to show all packages when using args filter Paired-with: @isaacs --- lib/ls.js | 80 ++++++++++++++++++++++++++++++++++----------------------------- 1 file changed, 43 insertions(+), 37 deletions(-) (limited to 'lib/ls.js') diff --git a/lib/ls.js b/lib/ls.js index 6f936a7b3..f73856c0a 100644 --- a/lib/ls.js +++ b/lib/ls.js @@ -16,6 +16,7 @@ const output = require('./utils/output.js') const _depth = Symbol('depth') const _dedupe = Symbol('dedupe') +const _filteredBy = Symbol('filteredBy') const _include = Symbol('include') const _invalid = Symbol('invalid') const _name = Symbol('name') @@ -78,12 +79,11 @@ const getProblems = (node, { global }) => { // item obj allowing for filtering out results during output const augmentItemWithIncludeMetadata = (node, item) => { item[_parent] = node[_parent] + item[_include] = node[_include] // append current item to its parent.nodes which is the // structure expected by archy in order to print tree - if (node[_include] && node[_parent]) { - item[_include] = true - + if (node[_include]) { // includes all ancestors of included node let p = node[_parent] while (p) { @@ -94,7 +94,7 @@ const augmentItemWithIncludeMetadata = (node, item) => { return item } -const getHumanOutputItem = (node, { color, global, long }) => { +const getHumanOutputItem = (node, { args, color, global, long }) => { const { pkgid, path } = node let printable = pkgid @@ -108,6 +108,8 @@ const getHumanOutputItem = (node, { color, global, long }) => { } } + const highlightDepName = + color && args.length && node[_filteredBy] const missingColor = isOptional(node) ? chalk.yellow.bgBlack : chalk.red.bgBlack @@ -118,7 +120,7 @@ const getHumanOutputItem = (node, { color, global, long }) => { ? (color ? missingColor(missingMsg) : missingMsg) : '' ) + - `${printable}` + + `${highlightDepName ? chalk.yellow.bgBlack(printable) : printable}` + (node[_dedupe] ? ' deduped' : '') + ( node[_invalid] @@ -220,7 +222,7 @@ const mapEdgesToNodes = (edge) => { // if the edge is linking to a missing node, we go ahead // and create a new obj that will represent the missing node - if (edge.missing || (edge.optional && !edge.to)) { + if (edge.missing || (edge.optional && !node)) { const { name, spec } = edge const pkgid = `${name}@${spec}` node = { name, pkgid, [_missing]: edge.from.pkgid } @@ -233,9 +235,9 @@ const mapEdgesToNodes = (edge) => { return node } -const filterByPositionalArgs = (args, node) => +const filterByPositionalArgs = (args, { node }) => args.length > 0 ? args.some( - (spec) => node.satisfies && node.satisfies(spec) + (spec) => (node.satisfies(spec)) ) : true const augmentNodesWithMetadata = ({ @@ -248,14 +250,18 @@ const augmentNodesWithMetadata = ({ // if the original edge was a deduped dep, treeverse will fail to // revisit that node in tree traversal logic, so we make it so that // we have a diff obj for deduped nodes: - if (seenNodes.has(node)) { + if (seenNodes.has(node.path)) { node = { name: node.name, version: node.version, pkgid: node.pkgid, package: node.package, + path: node.path, [_dedupe]: true } + } else { + // keeps track of already seen nodes in order to check for dedupes + seenNodes.set(node.path, node) } // _parent is going to be a ref to a treeverse-visited node (returned from @@ -264,8 +270,10 @@ const augmentNodesWithMetadata = ({ node[_parent] = nodeResult // _include is the property that allow us to filter based on position args // e.g: `npm ls foo`, `npm ls simple-output@2` - node[_include] = - filterByPositionalArgs(args, node, { parseable }) + // _filteredBy is used to apply extra color info to the item that + // was used in args in order to filter + node[_filteredBy] = node[_include] = + filterByPositionalArgs(args, { node: seenNodes.get(node.path), seenNodes }) // _depth keeps track of how many levels deep tree traversal currently is // so that we can `npm ls --depth=1` node[_depth] = currentDepth + 1 @@ -276,11 +284,7 @@ const augmentNodesWithMetadata = ({ const sortAlphabetically = (a, b) => a.pkgid.localeCompare(b.pkgid) -const humanOutput = ({ color, result, seenItems, topLevelChildren, unicode }) => { - if (!topLevelChildren) { - result.nodes = ['(empty)'] - } - +const humanOutput = ({ color, result, seenItems, unicode }) => { // we need to traverse the entire tree in order to determine which items // should be included (since a nested transitive included dep will make it // so that all its ancestors should be displayed) @@ -291,6 +295,10 @@ const humanOutput = ({ color, result, seenItems, topLevelChildren, unicode }) => } } + if (!result.nodes.length) { + result.nodes = ['(empty)'] + } + const archyOutput = archy(result, '', { unicode }) return color ? chalk.reset(archyOutput) : archyOutput } @@ -329,7 +337,7 @@ const jsonOutput = ({ path, problems, result, rootError, seenItems }) => { const parseableOutput = ({ global, long, seenNodes }) => { let out = '' - for (const node of seenNodes) { + for (const node of seenNodes.values()) { if (node.path && node[_include]) { out += node.path if (long) { @@ -378,10 +386,12 @@ const ls = async (args) => { }) const seenItems = new Set() - const seenNodes = new Set() + const seenNodes = new Map() const problems = new Set() - let topLevelChildren = 0 - const depthToPrint = all ? Infinity : (depth || 0) + const depthToPrint = (all || args.length) ? Infinity : (depth || 0) + + // add root node of tree to list of seenNodes + seenNodes.set(tree.path, tree) // tree traversal happens here, using treeverse.breadth const result = await breadth({ @@ -390,7 +400,9 @@ const ls = async (args) => { // the `tree` obj) that was just visited in the `visit` method below // `nodeResult` is going to be the returned `item` from `visit` getChildren (node, nodeResult) { - return (!(node instanceof Arborist.Node) || (node[_depth] > depthToPrint)) + const shouldSkipChildren = + !(node instanceof Arborist.Node) || (node[_depth] > depthToPrint) + return (shouldSkipChildren) ? [] : [...node.edgesOut.values()] .filter(filterByEdgesTypes({ @@ -405,6 +417,7 @@ const ls = async (args) => { })) .map(mapEdgesToNodes) .concat(appendExtraneousChildren({ node })) + .sort(sortAlphabetically) .map(augmentNodesWithMetadata({ args, currentDepth: node[_depth], @@ -412,40 +425,28 @@ const ls = async (args) => { parseable, seenNodes })) - .sort(sortAlphabetically) }, // visit each `node` of the `tree`, returning an `item` - these are // the elements that will be used to build the final output visit (node) { - seenNodes.add(node) - const nodeProblems = getProblems(node, { global }) for (const problem of nodeProblems) { problems.add(problem) } - // keeps track of the number of top-level children found since - // we have a bunch of edge cases related to empty top-level - if (node[_include] && node[_parent] && !node[_parent][_parent]) { - topLevelChildren++ - } - const item = json ? getJsonOutputItem(node, { global, long, nodeProblems }) : parseable ? null - : getHumanOutputItem(node, { color, global, long }) + : getHumanOutputItem(node, { args, color, global, long }) seenItems.add(item) + + // return a promise so we don't blow the stack return Promise.resolve(item) } }) - // if filtering items, should exit with error code on no results - if (!topLevelChildren && args.length) { - process.exitCode = 1 - } - // handle the special case of a broken package.json in the root folder const [rootError] = tree.errors.filter(e => e.code === 'EJSONPARSE' && e.path === resolve(path, 'package.json')) @@ -455,9 +456,14 @@ const ls = async (args) => { ? jsonOutput({ path, problems, result, rootError, seenItems }) : parseable ? parseableOutput({ seenNodes, global, long }) - : humanOutput({ color, result, seenItems, topLevelChildren, unicode }) + : humanOutput({ color, result, seenItems, unicode }) ) + // if filtering items, should exit with error code on no results + if (!tree[_include] && args.length) { + process.exitCode = 1 + } + if (rootError) { throw Object.assign( new Error('Failed to parse root package.json'), -- cgit v1.2.3