From 3320f0c241bfee1a45cbe1cec665adca655af885 Mon Sep 17 00:00:00 2001 From: nlf Date: Mon, 15 Aug 2022 13:31:10 -0700 Subject: [PATCH 1/5] feat(arborist): add overridden getter to Node class --- workspaces/arborist/lib/node.js | 4 +++ workspaces/arborist/test/node.js | 46 ++++++++++++++++++++++++++++++++ 2 files changed, 50 insertions(+) diff --git a/workspaces/arborist/lib/node.js b/workspaces/arborist/lib/node.js index be973565750a..8ec90ff3c849 100644 --- a/workspaces/arborist/lib/node.js +++ b/workspaces/arborist/lib/node.js @@ -334,6 +334,10 @@ class Node { return `${myname}@${alias}${version}` } + get overridden () { + return !!(this.overrides && this.overrides.value && this.overrides.name === this.name) + } + get package () { return this[_package] } diff --git a/workspaces/arborist/test/node.js b/workspaces/arborist/test/node.js index 0db439cbabc3..d89bf804a509 100644 --- a/workspaces/arborist/test/node.js +++ b/workspaces/arborist/test/node.js @@ -2728,6 +2728,52 @@ t.test('overrides', (t) => { t.end() }) + t.test('node.overridden is true when an override applies to a specific node', async (t) => { + const tree = new Node({ + loadOverrides: true, + path: '/some/path', + pkg: { + name: 'foo', + dependencies: { + bar: '^1', + }, + overrides: { + baz: '1.0.0', + }, + }, + children: [{ + name: 'bar', + version: '1.0.0', + pkg: { + dependencies: { + baz: '2.0.0', + }, + }, + children: [{ + name: 'baz', + version: '1.0.0', + pkg: { + dependencies: { + buzz: '1.0.0', + }, + }, + children: [{ + name: 'buzz', + version: '1.0.0', + pkg: {}, + }], + }], + }], + }) + + const bar = tree.edgesOut.get('bar').to + t.not(bar.overridden, 'bar was not overridden') + const baz = bar.edgesOut.get('baz').to + t.ok(baz.overridden, 'baz was overridden') + const buzz = baz.edgesOut.get('buzz').to + t.not(buzz.overridden, 'buzz was not overridden') + }) + t.test('assertRootOverrides throws when a dependency and override conflict', async (t) => { const conflictingTree = new Node({ loadOverrides: true, From bd6b9b0e6219b2b2094312931390d14de41b4b34 Mon Sep 17 00:00:00 2001 From: nlf Date: Mon, 15 Aug 2022 13:55:07 -0700 Subject: [PATCH 2/5] fix(ls): display overridden nodes --- lib/commands/ls.js | 13 + .../test/lib/commands/ls.js.test.cjs | 20 + test/lib/commands/ls.js | 342 +++++++++++++++++- 3 files changed, 363 insertions(+), 12 deletions(-) diff --git a/lib/commands/ls.js b/lib/commands/ls.js index 073ca0c6992e..6812c3923787 100644 --- a/lib/commands/ls.js +++ b/lib/commands/ls.js @@ -329,6 +329,11 @@ const getHumanOutputItem = (node, { args, color, global, long }) => { ? ' ' + (color ? chalk.green.bgBlack('extraneous') : 'extraneous') : '' ) + + ( + node.overridden + ? ' ' + (color ? chalk.gray('overridden') : 'overridden') + : '' + ) + (isGitNode(node) ? ` (${node.resolved})` : '') + (node.isLink ? ` -> ${relativePrefix}${targetLocation}` : '') + (long ? `${EOL}${node.package.description || ''}` : '') @@ -347,6 +352,13 @@ const getJsonOutputItem = (node, { global, long }) => { item.resolved = node.resolved } + // if the node is the project root, do not add the overridden flag. the project root can't be + // overridden anyway, and if we add the flag it causes undesirable behavior when `npm ls --json` + // is ran in an empty directory since we end up printing an object with only an overridden prop + if (!node.isProjectRoot) { + item.overridden = node.overridden + } + item[_name] = node.name // special formatting for top-level package name @@ -555,6 +567,7 @@ const parseableOutput = ({ global, long, seenNodes }) => { out += node.path !== node.realpath ? `:${node.realpath}` : '' out += isExtraneous(node, { global }) ? ':EXTRANEOUS' : '' out += node[_invalid] ? ':INVALID' : '' + out += node.overridden ? ':OVERRIDDEN' : '' } out += EOL } diff --git a/tap-snapshots/test/lib/commands/ls.js.test.cjs b/tap-snapshots/test/lib/commands/ls.js.test.cjs index 9b6749acfe3b..f511dec7cf20 100644 --- a/tap-snapshots/test/lib/commands/ls.js.test.cjs +++ b/tap-snapshots/test/lib/commands/ls.js.test.cjs @@ -255,6 +255,12 @@ exports[`test/lib/commands/ls.js TAP ls --parseable no args > should output pars {CWD}/tap-testdir-ls-ls---parseable-no-args/node_modules/dog ` +exports[`test/lib/commands/ls.js TAP ls --parseable overridden dep > should contain overridden outout 1`] = ` +{CWD}/tap-testdir-ls-ls---parseable-overridden-dep:test-overridden@1.0.0 +{CWD}/tap-testdir-ls-ls---parseable-overridden-dep/node_modules/foo:foo@1.0.0 +{CWD}/tap-testdir-ls-ls---parseable-overridden-dep/node_modules/bar:bar@1.0.0:OVERRIDDEN +` + exports[`test/lib/commands/ls.js TAP ls --parseable resolved points to git ref > should output tree containing git refs 1`] = ` {CWD}/tap-testdir-ls-ls---parseable-resolved-points-to-git-ref {CWD}/tap-testdir-ls-ls---parseable-resolved-points-to-git-ref/node_modules/abbrev @@ -567,6 +573,20 @@ test-npm-ls@1.0.0 {CWD}/tap-testdir-ls-ls-no-args ` +exports[`test/lib/commands/ls.js TAP ls overridden dep > should contain overridden outout 1`] = ` +test-overridden@1.0.0 {CWD}/tap-testdir-ls-ls-overridden-dep +\`-- foo@1.0.0 + \`-- bar@1.0.0 overridden + +` + +exports[`test/lib/commands/ls.js TAP ls overridden dep w/ color > should contain overridden outout 1`] = ` +test-overridden@1.0.0 {CWD}/tap-testdir-ls-ls-overridden-dep-w-color +\`-- foo@1.0.0 + \`-- bar@1.0.0 overridden + +` + exports[`test/lib/commands/ls.js TAP ls print deduped symlinks > should output tree containing linked deps 1`] = ` print-deduped-symlinks@1.0.0 {CWD}/tap-testdir-ls-ls-print-deduped-symlinks +-- a@1.0.0 diff --git a/test/lib/commands/ls.js b/test/lib/commands/ls.js index f4cd4ef33d58..764d95298ca1 100644 --- a/test/lib/commands/ls.js +++ b/test/lib/commands/ls.js @@ -231,6 +231,88 @@ t.test('ls', t => { t.matchSnapshot(redactCwd(result), 'should output containing problems info') }) + t.test('overridden dep', async t => { + config.all = true + t.teardown(() => { + config.all = false + }) + + npm.prefix = t.testdir({ + 'package.json': JSON.stringify({ + name: 'test-overridden', + version: '1.0.0', + dependencies: { + foo: '^1.0.0', + }, + overrides: { + bar: '1.0.0', + }, + }), + node_modules: { + foo: { + 'package.json': JSON.stringify({ + name: 'foo', + version: '1.0.0', + dependencies: { + bar: '^2.0.0', + }, + }), + }, + bar: { + 'package.json': JSON.stringify({ + name: 'bar', + version: '1.0.0', + }), + }, + }, + }) + + await ls.exec([]) + t.matchSnapshot(redactCwd(result), 'should contain overridden outout') + }) + + t.test('overridden dep w/ color', async t => { + config.all = true + npm.color = true + t.teardown(() => { + config.all = false + npm.color = false + }) + + npm.prefix = t.testdir({ + 'package.json': JSON.stringify({ + name: 'test-overridden', + version: '1.0.0', + dependencies: { + foo: '^1.0.0', + }, + overrides: { + bar: '1.0.0', + }, + }), + node_modules: { + foo: { + 'package.json': JSON.stringify({ + name: 'foo', + version: '1.0.0', + dependencies: { + bar: '^2.0.0', + }, + }), + }, + bar: { + 'package.json': JSON.stringify({ + name: 'bar', + version: '1.0.0', + }), + }, + }, + }) + + await ls.exec([]) + t.matchSnapshot(redactCwd(result), 'should contain overridden outout') + }) + t.test('with filter arg', async t => { npm.color = true npm.prefix = t.testdir({ @@ -1621,6 +1703,47 @@ t.test('ls --parseable', t => { t.matchSnapshot(redactCwd(result), 'should output containing problems info') }) + t.test('overridden dep', async t => { + config.all = true + config.long = true + t.teardown(() => { + config.all = false + config.long = false + }) + npm.prefix = t.testdir({ + 'package.json': JSON.stringify({ + name: 'test-overridden', + version: '1.0.0', + dependencies: { + foo: '^1.0.0', + }, + overrides: { + bar: '1.0.0', + }, + }), + node_modules: { + foo: { + 'package.json': JSON.stringify({ + name: 'foo', + version: '1.0.0', + dependencies: { + bar: '^2.0.0', + }, + }), + }, + bar: { + 'package.json': JSON.stringify({ + name: 'bar', + version: '1.0.0', + }), + }, + }, + }) + + await ls.exec([]) + t.matchSnapshot(redactCwd(result), 'should contain overridden outout') + }) + t.test('with filter arg', async t => { npm.prefix = t.testdir({ 'package.json': JSON.stringify({ @@ -2413,14 +2536,17 @@ t.test('ls --json', t => { dependencies: { foo: { version: '1.0.0', + overridden: false, dependencies: { dog: { version: '1.0.0', + overridden: false, }, }, }, chai: { version: '1.0.0', + overridden: false, }, }, }, @@ -2448,6 +2574,7 @@ t.test('ls --json', t => { dog: { version: '1.0.0', extraneous: true, + overridden: false, problems: [ /* eslint-disable-next-line max-len */ 'extraneous: dog@1.0.0 {CWD}/tap-testdir-ls-ls---json-missing-package.json/node_modules/dog', @@ -2456,6 +2583,7 @@ t.test('ls --json', t => { foo: { version: '1.0.0', extraneous: true, + overridden: false, problems: [ /* eslint-disable-next-line max-len */ 'extraneous: foo@1.0.0 {CWD}/tap-testdir-ls-ls---json-missing-package.json/node_modules/foo', @@ -2469,6 +2597,7 @@ t.test('ls --json', t => { chai: { version: '1.0.0', extraneous: true, + overridden: false, problems: [ /* eslint-disable-next-line max-len */ 'extraneous: chai@1.0.0 {CWD}/tap-testdir-ls-ls---json-missing-package.json/node_modules/chai', @@ -2503,15 +2632,18 @@ t.test('ls --json', t => { dependencies: { foo: { version: '1.0.0', + overridden: false, dependencies: { dog: { version: '1.0.0', + overridden: false, }, }, }, chai: { version: '1.0.0', extraneous: true, + overridden: false, problems: [ /* eslint-disable-next-line max-len */ 'extraneous: chai@1.0.0 {CWD}/tap-testdir-ls-ls---json-extraneous-deps/node_modules/chai', @@ -2523,6 +2655,58 @@ t.test('ls --json', t => { ) }) + t.test('overridden dep', async t => { + config.all = true + t.teardown(() => config.all = false) + npm.prefix = t.testdir({ + 'package.json': JSON.stringify({ + name: 'test-overridden', + version: '1.0.0', + dependencies: { + foo: '^1.0.0', + }, + overrides: { + bar: '1.0.0', + }, + }), + node_modules: { + foo: { + 'package.json': JSON.stringify({ + name: 'foo', + version: '1.0.0', + dependencies: { + bar: '^2.0.0', + }, + }), + }, + bar: { + 'package.json': JSON.stringify({ + name: 'bar', + version: '1.0.0', + }), + }, + }, + }) + + await ls.exec([]) + t.same(JSON.parse(result), { + name: 'test-overridden', + version: '1.0.0', + dependencies: { + foo: { + version: '1.0.0', + overridden: false, + dependencies: { + bar: { + version: '1.0.0', + overridden: true, + }, + }, + }, + }, + }) + }) + t.test('missing deps --long', async t => { t.plan(3) config.long = true @@ -2581,6 +2765,7 @@ t.test('ls --json', t => { dependencies: { chai: { version: '1.0.0', + overridden: false, }, }, }, @@ -2610,9 +2795,11 @@ t.test('ls --json', t => { dependencies: { foo: { version: '1.0.0', + overridden: false, dependencies: { dog: { version: '1.0.0', + overridden: false, }, }, }, @@ -2653,14 +2840,17 @@ t.test('ls --json', t => { dependencies: { foo: { version: '1.0.0', + overridden: false, dependencies: { dog: { version: '1.0.0', + overridden: false, }, }, }, chai: { version: '1.0.0', + overridden: false, }, }, }, @@ -2717,9 +2907,11 @@ t.test('ls --json', t => { dependencies: { foo: { version: '1.0.0', + overridden: false, }, chai: { version: '1.0.0', + overridden: false, }, }, }, @@ -2752,9 +2944,11 @@ t.test('ls --json', t => { dependencies: { foo: { version: '1.0.0', + overridden: false, }, chai: { version: '1.0.0', + overridden: false, }, }, }, @@ -2787,14 +2981,17 @@ t.test('ls --json', t => { dependencies: { foo: { version: '1.0.0', + overridden: false, dependencies: { dog: { version: '1.0.0', + overridden: false, }, }, }, chai: { version: '1.0.0', + overridden: false, }, }, }, @@ -2833,6 +3030,7 @@ t.test('ls --json', t => { foo: { version: '1.0.0', invalid: '"^2.0.0" from the root project', + overridden: false, problems: [ /* eslint-disable-next-line max-len */ 'invalid: foo@1.0.0 {CWD}/tap-testdir-ls-ls---json-missing-invalid-extraneous/node_modules/foo', @@ -2840,12 +3038,14 @@ t.test('ls --json', t => { dependencies: { dog: { version: '1.0.0', + overridden: false, }, }, }, chai: { version: '1.0.0', extraneous: true, + overridden: false, problems: [ /* eslint-disable-next-line max-len */ 'extraneous: chai@1.0.0 {CWD}/tap-testdir-ls-ls---json-missing-invalid-extraneous/node_modules/chai', @@ -2893,10 +3093,17 @@ t.test('ls --json', t => { dependencies: { 'dev-dep': { version: '1.0.0', + overridden: false, dependencies: { foo: { version: '1.0.0', - dependencies: { dog: { version: '1.0.0' } }, + overridden: false, + dependencies: { + dog: { + version: '1.0.0', + overridden: false, + }, + }, }, }, }, @@ -2949,6 +3156,7 @@ t.test('ls --json', t => { 'linked-dep': { version: '1.0.0', resolved: 'file:../linked-dep', + overridden: false, }, }, }, @@ -2986,9 +3194,24 @@ t.test('ls --json', t => { name: 'test-npm-ls', version: '1.0.0', dependencies: { - chai: { version: '1.0.0' }, - 'optional-dep': { version: '1.0.0' }, - 'prod-dep': { version: '1.0.0', dependencies: { dog: { version: '2.0.0' } } }, + chai: { + version: '1.0.0', + overridden: false, + }, + 'optional-dep': { + version: '1.0.0', + overridden: false, + }, + 'prod-dep': { + version: '1.0.0', + overridden: false, + dependencies: { + dog: { + version: '2.0.0', + overridden: false, + }, + }, + }, }, }, 'should output json containing production deps' @@ -3111,6 +3334,7 @@ t.test('ls --json', t => { dependencies: { '@isaacs/dedupe-tests-a': { version: '1.0.1', + overridden: false, resolved: 'https://registry.npmjs.org/@isaacs/dedupe-tests-a/-/dedupe-tests-a-1.0.1.tgz', dependencies: { @@ -3118,6 +3342,7 @@ t.test('ls --json', t => { resolved: 'https://registry.npmjs.org/@isaacs/dedupe-tests-b/-/dedupe-tests-b-1.0.0.tgz', extraneous: true, + overridden: false, problems: [ /* eslint-disable-next-line max-len */ 'extraneous: @isaacs/dedupe-tests-b@ {CWD}/tap-testdir-ls-ls---json-from-lockfile/node_modules/@isaacs/dedupe-tests-a/node_modules/@isaacs/dedupe-tests-b', @@ -3127,6 +3352,7 @@ t.test('ls --json', t => { }, '@isaacs/dedupe-tests-b': { version: '2.0.0', + overridden: false, resolved: 'https://registry.npmjs.org/@isaacs/dedupe-tests-b/-/dedupe-tests-b-2.0.0.tgz', }, @@ -3171,6 +3397,7 @@ t.test('ls --json', t => { dependencies: { 'peer-dep': { name: 'peer-dep', + overridden: false, description: 'Peer-dep description here', version: '1.0.0', _id: 'peer-dep@1.0.0', @@ -3182,15 +3409,18 @@ t.test('ls --json', t => { }, 'dev-dep': { name: 'dev-dep', + overridden: false, description: 'A DEV dep kind of dep', version: '1.0.0', dependencies: { foo: { name: 'foo', version: '1.0.0', + overridden: false, dependencies: { dog: { name: 'dog', + overridden: false, version: '1.0.0', _id: 'dog@1.0.0', devDependencies: {}, @@ -3217,6 +3447,7 @@ t.test('ls --json', t => { }, chai: { name: 'chai', + overridden: false, version: '1.0.0', _id: 'chai@1.0.0', devDependencies: {}, @@ -3227,6 +3458,7 @@ t.test('ls --json', t => { }, 'optional-dep': { name: 'optional-dep', + overridden: false, description: 'Maybe a dep?', version: '1.0.0', _id: 'optional-dep@1.0.0', @@ -3238,11 +3470,13 @@ t.test('ls --json', t => { }, 'prod-dep': { name: 'prod-dep', + overridden: false, description: 'A PROD dep kind of dep', version: '1.0.0', dependencies: { dog: { name: 'dog', + overridden: false, description: 'A dep that bars', version: '2.0.0', _id: 'dog@2.0.0', @@ -3308,6 +3542,7 @@ t.test('ls --json', t => { dependencies: { 'peer-dep': { name: 'peer-dep', + overridden: false, description: 'Peer-dep description here', version: '1.0.0', _id: 'peer-dep@1.0.0', @@ -3319,6 +3554,7 @@ t.test('ls --json', t => { }, 'dev-dep': { name: 'dev-dep', + overridden: false, description: 'A DEV dep kind of dep', version: '1.0.0', _id: 'dev-dep@1.0.0', @@ -3330,6 +3566,7 @@ t.test('ls --json', t => { }, chai: { name: 'chai', + overridden: false, version: '1.0.0', _id: 'chai@1.0.0', devDependencies: {}, @@ -3340,6 +3577,7 @@ t.test('ls --json', t => { }, 'optional-dep': { name: 'optional-dep', + overridden: false, description: 'Maybe a dep?', version: '1.0.0', _id: 'optional-dep@1.0.0', @@ -3351,6 +3589,7 @@ t.test('ls --json', t => { }, 'prod-dep': { name: 'prod-dep', + overridden: false, description: 'A PROD dep kind of dep', version: '1.0.0', _id: 'prod-dep@1.0.0', @@ -3439,6 +3678,7 @@ t.test('ls --json', t => { 'peer-dep': { version: '1.0.0', invalid: '"^2.0.0" from the root project', + overridden: false, problems: [ /* eslint-disable-next-line max-len */ 'invalid: peer-dep@1.0.0 {CWD}/tap-testdir-ls-ls---json-unmet-peer-dep/node_modules/peer-dep', @@ -3446,16 +3686,38 @@ t.test('ls --json', t => { }, 'dev-dep': { version: '1.0.0', + overridden: false, dependencies: { foo: { version: '1.0.0', - dependencies: { dog: { version: '1.0.0' } }, + overridden: false, + dependencies: { + dog: { + version: '1.0.0', + overridden: false, + }, + }, + }, + }, + }, + chai: { + version: '1.0.0', + overridden: false, + }, + 'optional-dep': { + version: '1.0.0', + overridden: false, + }, + 'prod-dep': { + version: '1.0.0', + overridden: false, + dependencies: { + dog: { + version: '2.0.0', + overridden: false, }, }, }, - chai: { version: '1.0.0' }, - 'optional-dep': { version: '1.0.0' }, - 'prod-dep': { version: '1.0.0', dependencies: { dog: { version: '2.0.0' } } }, }, }, 'should output json signaling missing peer dep in problems' @@ -3502,6 +3764,7 @@ t.test('ls --json', t => { 'optional-dep': { version: '1.0.0', invalid: '"^2.0.0" from the root project', + overridden: false, problems: [ /* eslint-disable-next-line max-len */ 'invalid: optional-dep@1.0.0 {CWD}/tap-testdir-ls-ls---json-unmet-optional-dep/node_modules/optional-dep', @@ -3509,18 +3772,38 @@ t.test('ls --json', t => { }, 'peer-dep': { version: '1.0.0', + overridden: false, }, 'dev-dep': { version: '1.0.0', + overridden: false, dependencies: { foo: { version: '1.0.0', - dependencies: { dog: { version: '1.0.0' } }, + overridden: false, + dependencies: { + dog: { + version: '1.0.0', + overridden: false, + }, + }, + }, + }, + }, + chai: { + version: '1.0.0', + overridden: false, + }, + 'prod-dep': { + version: '1.0.0', + overridden: false, + dependencies: { + dog: { + version: '2.0.0', + overridden: false, }, }, }, - chai: { version: '1.0.0' }, - 'prod-dep': { version: '1.0.0', dependencies: { dog: { version: '2.0.0' } } }, 'missing-optional-dep': {}, // missing optional dep has an empty entry in json output }, }, @@ -3567,11 +3850,15 @@ t.test('ls --json', t => { dependencies: { a: { version: '1.0.0', + overridden: false, dependencies: { b: { version: '1.0.0', + overridden: false, dependencies: { - a: { version: '1.0.0' }, + a: { + version: '1.0.0', + }, }, }, }, @@ -3623,6 +3910,7 @@ t.test('ls --json', t => { dependencies: { a: { version: '1.0.0', + overridden: false, resolved: 'https://localhost:8080/abbrev/-/abbrev-1.1.1.tgz', }, }, @@ -3683,6 +3971,7 @@ t.test('ls --json', t => { dependencies: { abbrev: { version: '1.1.1', + overridden: false, /* eslint-disable-next-line max-len */ resolved: 'git+ssh://git@github.com/isaacs/abbrev-js.git#b8f3a2fc0c3bb8ffd8b0d0072cc6b5a3667e963c', }, @@ -3760,6 +4049,7 @@ t.test('ls --json', t => { dependencies: { 'simple-output': { version: '2.1.1', + overridden: false, resolved: 'https://registry.npmjs.org/simple-output/-/simple-output-2.1.1.tgz', }, }, @@ -3823,12 +4113,15 @@ t.test('ls --json', t => { dependencies: { a: { version: '1.0.0', + overridden: false, }, b: { version: '1.0.0', + overridden: false, dependencies: { c: { version: '1.0.0', + overridden: false, }, }, }, @@ -3934,14 +4227,17 @@ t.test('ls --package-lock-only', t => { dependencies: { foo: { version: '1.0.0', + overridden: false, dependencies: { dog: { version: '1.0.0', + overridden: false, }, }, }, chai: { version: '1.0.0', + overridden: false, }, }, }, @@ -3984,9 +4280,11 @@ t.test('ls --package-lock-only', t => { dependencies: { foo: { version: '1.0.0', + overridden: false, dependencies: { dog: { version: '1.0.0', + overridden: false, }, }, }, @@ -4080,6 +4378,7 @@ t.test('ls --package-lock-only', t => { dependencies: { chai: { version: '1.0.0', + overridden: false, }, }, }, @@ -4127,9 +4426,11 @@ t.test('ls --package-lock-only', t => { dependencies: { foo: { version: '1.0.0', + overridden: false, dependencies: { dog: { version: '1.0.0', + overridden: false, }, }, }, @@ -4179,14 +4480,17 @@ t.test('ls --package-lock-only', t => { dependencies: { foo: { version: '1.0.0', + overridden: false, dependencies: { dog: { version: '1.0.0', + overridden: false, }, }, }, chai: { version: '1.0.0', + overridden: false, }, }, }, @@ -4273,9 +4577,11 @@ t.test('ls --package-lock-only', t => { dependencies: { foo: { version: '1.0.0', + overridden: false, }, chai: { version: '1.0.0', + overridden: false, }, }, }, @@ -4323,9 +4629,11 @@ t.test('ls --package-lock-only', t => { dependencies: { foo: { version: '1.0.0', + overridden: false, }, chai: { version: '1.0.0', + overridden: false, }, }, }, @@ -4373,14 +4681,17 @@ t.test('ls --package-lock-only', t => { dependencies: { foo: { version: '1.0.0', + overridden: false, dependencies: { dog: { version: '1.0.0', + overridden: false, }, }, }, chai: { version: '1.0.0', + overridden: false, }, }, }, @@ -4431,6 +4742,7 @@ t.test('ls --package-lock-only', t => { dependencies: { foo: { version: '1.0.0', + overridden: false, invalid: '"^2.0.0" from the root project', problems: [ /* eslint-disable-next-line max-len */ @@ -4439,6 +4751,7 @@ t.test('ls --package-lock-only', t => { dependencies: { dog: { version: '1.0.0', + overridden: false, }, }, }, @@ -4544,11 +4857,13 @@ t.test('ls --package-lock-only', t => { dependencies: { '@isaacs/dedupe-tests-a': { version: '1.0.1', + overridden: false, resolved: 'https://registry.npmjs.org/@isaacs/dedupe-tests-a/-/dedupe-tests-a-1.0.1.tgz', dependencies: { '@isaacs/dedupe-tests-b': { version: '1.0.0', + overridden: false, resolved: 'https://registry.npmjs.org/@isaacs/dedupe-tests-b/-/dedupe-tests-b-1.0.0.tgz', }, @@ -4556,6 +4871,7 @@ t.test('ls --package-lock-only', t => { }, '@isaacs/dedupe-tests-b': { version: '2.0.0', + overridden: false, resolved: 'https://registry.npmjs.org/@isaacs/dedupe-tests-b/-/dedupe-tests-b-2.0.0.tgz', }, @@ -4592,6 +4908,7 @@ t.test('ls --package-lock-only', t => { dependencies: { a: { version: '1.0.0', + overridden: false, resolved: 'https://localhost:8080/abbrev/-/abbrev-1.0.0.tgz', }, }, @@ -4634,6 +4951,7 @@ t.test('ls --package-lock-only', t => { abbrev: { /* eslint-disable-next-line max-len */ resolved: 'git+ssh://git@github.com/isaacs/abbrev-js.git#b8f3a2fc0c3bb8ffd8b0d0072cc6b5a3667e963c', + overridden: false, }, }, }, From 05d4682155666dd023bbb622e5e1a5706318b10f Mon Sep 17 00:00:00 2001 From: nlf Date: Tue, 16 Aug 2022 10:07:55 -0700 Subject: [PATCH 3/5] fix(explain): display override information --- lib/commands/explain.js | 3 ++- lib/utils/explain-dep.js | 17 +++++++++++--- .../test/lib/utils/explain-dep.js.test.cjs | 22 +++++++++++++++++++ test/lib/utils/explain-dep.js | 17 ++++++++++++++ 4 files changed, 55 insertions(+), 4 deletions(-) diff --git a/lib/commands/explain.js b/lib/commands/explain.js index c0ef04548a4e..a06ad24152a1 100644 --- a/lib/commands/explain.js +++ b/lib/commands/explain.js @@ -59,7 +59,7 @@ class Explain extends ArboristWorkspaceCmd { const expls = [] for (const node of nodes) { - const { extraneous, dev, optional, devOptional, peer, inBundle } = node + const { extraneous, dev, optional, devOptional, peer, inBundle, overridden } = node const expl = node.explain() if (extraneous) { expl.extraneous = true @@ -69,6 +69,7 @@ class Explain extends ArboristWorkspaceCmd { expl.devOptional = devOptional expl.peer = peer expl.bundled = inBundle + expl.overridden = overridden } expls.push(expl) } diff --git a/lib/utils/explain-dep.js b/lib/utils/explain-dep.js index 107f68549ef1..cd53a2269640 100644 --- a/lib/utils/explain-dep.js +++ b/lib/utils/explain-dep.js @@ -8,6 +8,7 @@ const nocolor = { magenta: s => s, blue: s => s, green: s => s, + gray: s => s, } const { relative } = require('path') @@ -18,13 +19,14 @@ const explainNode = (node, depth, color) => explainLinksIn(node, depth, color) const colorType = (type, color) => { - const { red, yellow, cyan, magenta, blue, green } = color ? chalk : nocolor + const { red, yellow, cyan, magenta, blue, green, gray } = color ? chalk : nocolor const style = type === 'extraneous' ? red : type === 'dev' ? yellow : type === 'optional' ? cyan : type === 'peer' ? magenta : type === 'bundled' ? blue : type === 'workspace' ? green + : type === 'overridden' ? gray : /* istanbul ignore next */ s => s return style(type) } @@ -40,6 +42,7 @@ const printNode = (node, color) => { peer, bundled, isWorkspace, + overridden, } = node const { bold, dim, green } = color ? chalk : nocolor const extra = [] @@ -63,6 +66,10 @@ const printNode = (node, color) => { extra.push(' ' + bold(colorType('bundled', color))) } + if (overridden) { + extra.push(' ' + bold(colorType('overridden', color))) + } + const pkgid = isWorkspace ? green(`${name}@${version}`) : `${bold(name)}@${bold(version)}` @@ -112,11 +119,15 @@ const explainDependents = ({ name, dependents }, depth, color) => { return str.split('\n').join('\n ') } -const explainEdge = ({ name, type, bundled, from, spec }, depth, color) => { +const explainEdge = ({ name, type, bundled, from, spec, rawSpec, overridden }, depth, color) => { const { bold } = color ? chalk : nocolor - const dep = type === 'workspace' + let dep = type === 'workspace' ? bold(relative(from.location, spec.slice('file:'.length))) : `${bold(name)}@"${bold(spec)}"` + if (overridden) { + dep = `${colorType('overridden', color)} ${dep} (was "${rawSpec}")` + } + const fromMsg = ` from ${explainFrom(from, depth, color)}` return (type === 'prod' ? '' : `${colorType(type, color)} `) + diff --git a/tap-snapshots/test/lib/utils/explain-dep.js.test.cjs b/tap-snapshots/test/lib/utils/explain-dep.js.test.cjs index 4d6f4686df8c..8550617eb0a0 100644 --- a/tap-snapshots/test/lib/utils/explain-dep.js.test.cjs +++ b/tap-snapshots/test/lib/utils/explain-dep.js.test.cjs @@ -156,6 +156,28 @@ optdep@1.0.0 optional node_modules/optdep ` +exports[`test/lib/utils/explain-dep.js TAP overridden > explain color deep 1`] = ` +overridden-root@1.0.0 overridden +node_modules/overridden-root + overridden overridden-dep@"1.0.0" (was "^2.0.0") from the root project +` + +exports[`test/lib/utils/explain-dep.js TAP overridden > explain nocolor shallow 1`] = ` +overridden-root@1.0.0 overridden +node_modules/overridden-root + overridden overridden-dep@"1.0.0" (was "^2.0.0") from the root project +` + +exports[`test/lib/utils/explain-dep.js TAP overridden > print color 1`] = ` +overridden-root@1.0.0 overridden +node_modules/overridden-root +` + +exports[`test/lib/utils/explain-dep.js TAP overridden > print nocolor 1`] = ` +overridden-root@1.0.0 overridden +node_modules/overridden-root +` + exports[`test/lib/utils/explain-dep.js TAP peer > explain color deep 1`] = ` peer@1.0.0 peer node_modules/peer diff --git a/test/lib/utils/explain-dep.js b/test/lib/utils/explain-dep.js index 000f5b8165a9..ed006c01d78f 100644 --- a/test/lib/utils/explain-dep.js +++ b/test/lib/utils/explain-dep.js @@ -129,6 +129,23 @@ const cases = { dependents: [], extraneous: true, }, + + overridden: { + name: 'overridden-root', + version: '1.0.0', + location: 'node_modules/overridden-root', + overridden: true, + dependents: [{ + type: 'prod', + name: 'overridden-dep', + spec: '1.0.0', + rawSpec: '^2.0.0', + overridden: true, + from: { + location: '/path/to/project', + }, + }], + }, } cases.manyDeps = { From 6f60380d14fcd2156afd3ecf63c3b6436bfa0d5d Mon Sep 17 00:00:00 2001 From: nlf Date: Tue, 16 Aug 2022 10:23:06 -0700 Subject: [PATCH 4/5] feat(arborist): add :overridden pseudo selector --- workspaces/arborist/lib/query-selector-all.js | 4 ++++ workspaces/arborist/test/query-selector-all.js | 8 ++++++-- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/workspaces/arborist/lib/query-selector-all.js b/workspaces/arborist/lib/query-selector-all.js index 6c540dea3c87..a3eac5ddc123 100644 --- a/workspaces/arborist/lib/query-selector-all.js +++ b/workspaces/arborist/lib/query-selector-all.js @@ -262,6 +262,10 @@ class Results { !internalSelector.has(node)) } + overriddenPseudo () { + return this.initialItems.filter(node => node.overridden) + } + pathPseudo () { return this.initialItems.filter(node => { if (!this.currentAstNode.pathValue) { diff --git a/workspaces/arborist/test/query-selector-all.js b/workspaces/arborist/test/query-selector-all.js index c335a82cbaf4..3bfe34bd8ef2 100644 --- a/workspaces/arborist/test/query-selector-all.js +++ b/workspaces/arborist/test/query-selector-all.js @@ -28,7 +28,7 @@ t.test('query-selector-all', async t => { │ └── moo@3.0.0 (production dep of bar) ├─┬ foo@2.2.2 (dev dep of query-selector-all-tests) │ ├─┬ bar@1.4.0 (production dep of foo, deduped) - │ │ └── dasher@2.0.0 (peer dep of bar) + │ │ └── dasher@2.0.0 (overridden peer dep of bar) │ └── dash-separated-pkg@1.0.0 (production dep of foo) ├── moo@3.0.0 (dev dep of query-selector-all-tests) └─┬ recur@1.0.0 (dev dep of query-selector-all-tests) @@ -98,7 +98,7 @@ t.test('query-selector-all', async t => { name: 'bar', version: '1.4.0', peerDependencies: { - dasher: '2.0.0', + dasher: '2.1.0', }, }), }, @@ -203,6 +203,9 @@ t.test('query-selector-all', async t => { bar: '^2.0.0', ipsum: 'npm:sit@1.0.0', }, + overrides: { + dasher: '2.0.0', + }, devDependencies: { foo: '^2.0.0', moo: '^3.0.0', @@ -368,6 +371,7 @@ t.test('query-selector-all', async t => { ]], [':missing', ['missing-dep@^1.0.0']], [':private', ['b@1.0.0']], + [':overridden', ['dasher@2.0.0']], // :not pseudo [':not(#foo)', [ From d13ad3a7e782b1e16535f2ca5a1c88c40943e7ec Mon Sep 17 00:00:00 2001 From: nlf Date: Tue, 16 Aug 2022 10:27:55 -0700 Subject: [PATCH 5/5] feat(query): support :overridden pseudo selector --- .../content/using-npm/dependency-selectors.md | 2 +- lib/commands/query.js | 1 + .../test/lib/commands/query.js.test.cjs | 33 ++++++++++++------- 3 files changed, 24 insertions(+), 12 deletions(-) diff --git a/docs/content/using-npm/dependency-selectors.md b/docs/content/using-npm/dependency-selectors.md index 45febf7cc730..c96057c798ef 100644 --- a/docs/content/using-npm/dependency-selectors.md +++ b/docs/content/using-npm/dependency-selectors.md @@ -54,7 +54,7 @@ The [`npm query`](/commands/npm-query) commmand exposes a new dependency selecto - [`:private`](https://docs.npmjs.com/cli/v8/configuring-npm/package-json#private) when a dependency is private - `:link` when a dependency is linked (for instance, workspaces or packages manually [`linked`](https://docs.npmjs.com/cli/v8/commands/npm-link) - `:deduped` when a dependency has been deduped (note that this does *not* always mean the dependency has been hoisted to the root of node_modules) -- `:override` when a dependency is an override (not implemented yet) +- `:overridden` when a dependency has been overridden - `:extraneous` when a dependency exists but is not defined as a dependency of any node - `:invalid` when a dependency version is out of its ancestors specified range - `:missing` when a dependency is not found on disk diff --git a/lib/commands/query.js b/lib/commands/query.js index 60294acaf4a6..231329b19b5c 100644 --- a/lib/commands/query.js +++ b/lib/commands/query.js @@ -20,6 +20,7 @@ class QuerySelectorItem { this.dev = node.target.dev this.inBundle = node.target.inBundle this.deduped = this.from.length > 1 + this.overridden = node.overridden for (const edge of node.target.edgesIn) { this.from.push(edge.from.location) } diff --git a/tap-snapshots/test/lib/commands/query.js.test.cjs b/tap-snapshots/test/lib/commands/query.js.test.cjs index 0a2aa769ee63..d827b62eef74 100644 --- a/tap-snapshots/test/lib/commands/query.js.test.cjs +++ b/tap-snapshots/test/lib/commands/query.js.test.cjs @@ -22,7 +22,8 @@ exports[`test/lib/commands/query.js TAP global > should return global package 1` "to": [], "dev": false, "inBundle": false, - "deduped": false + "deduped": false, + "overridden": false } ] ` @@ -51,7 +52,8 @@ exports[`test/lib/commands/query.js TAP include-workspace-root > should return w ], "dev": false, "inBundle": false, - "deduped": false + "deduped": false, + "overridden": false }, { "name": "c", @@ -66,7 +68,8 @@ exports[`test/lib/commands/query.js TAP include-workspace-root > should return w "to": [], "dev": false, "inBundle": false, - "deduped": false + "deduped": false, + "overridden": false } ] ` @@ -86,7 +89,8 @@ exports[`test/lib/commands/query.js TAP linked node > should return linked node "to": [], "dev": false, "inBundle": false, - "deduped": false + "deduped": false, + "overridden": false } ] ` @@ -111,7 +115,8 @@ exports[`test/lib/commands/query.js TAP recursive tree > should return everythin ], "dev": false, "inBundle": false, - "deduped": false + "deduped": false, + "overridden": false }, { "pkgid": "a@", @@ -125,7 +130,8 @@ exports[`test/lib/commands/query.js TAP recursive tree > should return everythin "to": [], "dev": false, "inBundle": false, - "deduped": false + "deduped": false, + "overridden": false }, { "pkgid": "b@", @@ -139,7 +145,8 @@ exports[`test/lib/commands/query.js TAP recursive tree > should return everythin "to": [], "dev": false, "inBundle": false, - "deduped": false + "deduped": false, + "overridden": false } ] ` @@ -167,7 +174,8 @@ exports[`test/lib/commands/query.js TAP simple query > should return root object ], "dev": false, "inBundle": false, - "deduped": false + "deduped": false, + "overridden": false }, { "pkgid": "a@", @@ -181,7 +189,8 @@ exports[`test/lib/commands/query.js TAP simple query > should return root object "to": [], "dev": false, "inBundle": false, - "deduped": false + "deduped": false, + "overridden": false }, { "pkgid": "b@", @@ -195,7 +204,8 @@ exports[`test/lib/commands/query.js TAP simple query > should return root object "to": [], "dev": false, "inBundle": false, - "deduped": false + "deduped": false, + "overridden": false } ] ` @@ -215,7 +225,8 @@ exports[`test/lib/commands/query.js TAP workspace query > should return workspac "to": [], "dev": false, "inBundle": false, - "deduped": false + "deduped": false, + "overridden": false } ] `