Skip to content

Commit

Permalink
ls: show relative paths from root, not absolute
Browse files Browse the repository at this point in the history
Change the output in `npm ls` for symlink dependencies (including
workspaces) to show the relative path from the project root, rather than
the absolute path of the link target.

This makes the output much less noisy and more ergonomic when many
workspaces and link dependencies are in use, especially when paths are
long.

It is arguable that this output might be slightly misleading, since the
_actual_ workspace symlink from `${root}/node_modules/b` to
`${root}/packages/b` has a link value of `../packages/b`, not just
`packages/b`.  (Or on Windows, always the full absolute path, because
junctions.)  Thus, `npm ls b` will not show the same output as `ls -l
node_modules/b`.

However, `npm ls` shows the logical tree, not the physical tree, so
presenting the user with a path that they can use and interpret is more
important than presenting them with the strictly accurate details of
their filesystem.
  • Loading branch information
isaacs committed May 20, 2021
1 parent c18626f commit 5f20292
Show file tree
Hide file tree
Showing 2 changed files with 29 additions and 25 deletions.
8 changes: 6 additions & 2 deletions lib/ls.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
const { resolve } = require('path')
const { resolve, relative } = require('path')
const { EOL } = require('os')

const archy = require('archy')
Expand Down Expand Up @@ -298,6 +298,9 @@ const getHumanOutputItem = (node, { args, color, global, long }) => {
? chalk.yellow.bgBlack
: chalk.red.bgBlack
const missingMsg = `UNMET ${isOptional(node) ? 'OPTIONAL ' : ''}DEPENDENCY`
const targetLocation = node.root
? relative(node.root.realpath, node.realpath)
: node.targetLocation
const label =
(
node[_missing]
Expand All @@ -321,7 +324,7 @@ const getHumanOutputItem = (node, { args, color, global, long }) => {
: ''
) +
(isGitNode(node) ? ` (${node.resolved})` : '') +
(node.isLink ? ` -> ${node.realpath}` : '') +
(node.isLink ? ` -> ${targetLocation}` : '') +
(long ? `${EOL}${node.package.description || ''}` : '')

return augmentItemWithIncludeMetadata(node, { label, nodes: [] })
Expand Down Expand Up @@ -453,6 +456,7 @@ const augmentNodesWithMetadata = ({
path: node.path,
isLink: node.isLink,
realpath: node.realpath,
targetLocation: node.root ? relative(node.root.realpath, node.realpath) : node.targetLocation,
[_type]: node[_type],
[_invalid]: node[_invalid],
[_missing]: node[_missing],
Expand Down
46 changes: 23 additions & 23 deletions tap-snapshots/test/lib/ls.js.test.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ [email protected] {CWD}/tap-testdir-ls-ls---dev

exports[`test/lib/ls.js TAP ls --link > should output tree containing linked deps 1`] = `
[email protected] {CWD}/tap-testdir-ls-ls---link
\`-- [email protected] -> {CWD}/tap-testdir-ls-ls---link/linked-dep
\`-- [email protected] -> linked-dep
`

Expand Down Expand Up @@ -480,59 +480,59 @@ 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`] = `
[email protected] {CWD}/tap-testdir-ls-ls-loading-a-tree-containing-workspaces
+-- [email protected] -> {CWD}/tap-testdir-ls-ls-loading-a-tree-containing-workspaces/group/e
\`-- [email protected] -> {CWD}/tap-testdir-ls-ls-loading-a-tree-containing-workspaces/group/f
+-- [email protected] -> group/e
\`-- [email protected] -> group/f
`

exports[`test/lib/ls.js TAP ls loading a tree containing workspaces > should filter single workspace 1`] = `
[email protected] {CWD}/tap-testdir-ls-ls-loading-a-tree-containing-workspaces
+-- [email protected] -> {CWD}/tap-testdir-ls-ls-loading-a-tree-containing-workspaces/a
| \`-- [email protected] deduped -> {CWD}/tap-testdir-ls-ls-loading-a-tree-containing-workspaces/d
\`-- [email protected] -> {CWD}/tap-testdir-ls-ls-loading-a-tree-containing-workspaces/d
+-- [email protected] -> a
| \`-- [email protected] deduped -> d
\`-- [email protected] -> d
`

exports[`test/lib/ls.js TAP ls loading a tree containing workspaces > should filter using workspace config 1`] = `
[email protected] {CWD}/tap-testdir-ls-ls-loading-a-tree-containing-workspaces
\`-- [email protected] -> {CWD}/tap-testdir-ls-ls-loading-a-tree-containing-workspaces/a
\`-- [email protected] -> a
+-- [email protected]
\`-- [email protected] -> {CWD}/tap-testdir-ls-ls-loading-a-tree-containing-workspaces/d
\`-- [email protected] -> d
\`-- [email protected]
\`-- [email protected]
`

exports[`test/lib/ls.js TAP ls loading a tree containing workspaces > should list --all workspaces properly 1`] = `
[email protected] {CWD}/tap-testdir-ls-ls-loading-a-tree-containing-workspaces
+-- [email protected] -> {CWD}/tap-testdir-ls-ls-loading-a-tree-containing-workspaces/a
+-- [email protected] -> a
| +-- [email protected]
| \`-- [email protected] deduped -> {CWD}/tap-testdir-ls-ls-loading-a-tree-containing-workspaces/d
+-- [email protected] -> {CWD}/tap-testdir-ls-ls-loading-a-tree-containing-workspaces/b
+-- [email protected] -> {CWD}/tap-testdir-ls-ls-loading-a-tree-containing-workspaces/d
| \`-- [email protected] deduped -> d
+-- [email protected] -> b
+-- [email protected] -> d
| \`-- [email protected]
| \`-- [email protected]
+-- [email protected] -> {CWD}/tap-testdir-ls-ls-loading-a-tree-containing-workspaces/group/e
\`-- [email protected] -> {CWD}/tap-testdir-ls-ls-loading-a-tree-containing-workspaces/group/f
+-- [email protected] -> group/e
\`-- [email protected] -> group/f
`

exports[`test/lib/ls.js TAP ls loading a tree containing workspaces > should list workspaces properly with default configs 1`] = `
[[email protected] {CWD}/tap-testdir-ls-ls-loading-a-tree-containing-workspaces
[0m+-- [[email protected][39m -> {CWD}/tap-testdir-ls-ls-loading-a-tree-containing-workspaces/a[0m
[0m+-- [[email protected][39m -> a[0m
| +-- [email protected]
[0m| \`-- [email protected] [90mdeduped[39m -> {CWD}/tap-testdir-ls-ls-loading-a-tree-containing-workspaces/d[0m
[0m+-- [[email protected][39m -> {CWD}/tap-testdir-ls-ls-loading-a-tree-containing-workspaces/b[0m
[0m+-- [[email protected][39m -> {CWD}/tap-testdir-ls-ls-loading-a-tree-containing-workspaces/d[0m
[0m| \`-- [email protected] [90mdeduped[39m -> d[0m
[0m+-- [[email protected][39m -> b[0m
[0m+-- [[email protected][39m -> d[0m
| \`-- [email protected]
[0m+-- [[email protected][39m -> {CWD}/tap-testdir-ls-ls-loading-a-tree-containing-workspaces/group/e[0m
[0m\`-- [[email protected][39m -> {CWD}/tap-testdir-ls-ls-loading-a-tree-containing-workspaces/group/f[0m
[0m+-- [[email protected][39m -> group/e[0m
[0m\`-- [[email protected][39m -> group/f[0m

`

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`] = `
[email protected] {CWD}/tap-testdir-ls-ls-loading-a-tree-containing-workspaces
\`-- [email protected] -> {CWD}/tap-testdir-ls-ls-loading-a-tree-containing-workspaces/d
\`-- [email protected] -> d
\`-- [email protected]
\`-- [email protected]
Expand Down Expand Up @@ -567,8 +567,8 @@ [email protected] {CWD}/tap-testdir-ls-ls-no-args
exports[`test/lib/ls.js TAP ls print deduped symlinks > should output tree containing linked deps 1`] = `
[email protected] {CWD}/tap-testdir-ls-ls-print-deduped-symlinks
+-- [email protected]
| \`-- [email protected] deduped -> {CWD}/tap-testdir-ls-ls-print-deduped-symlinks/b
\`-- [email protected] -> {CWD}/tap-testdir-ls-ls-print-deduped-symlinks/b
| \`-- [email protected] deduped -> b
\`-- [email protected] -> b
`

Expand Down

0 comments on commit 5f20292

Please sign in to comment.