diff --git a/lib/commands/view.js b/lib/commands/view.js index 6656df9873e2c..9a3d8f29233c4 100644 --- a/lib/commands/view.js +++ b/lib/commands/view.js @@ -240,18 +240,12 @@ class View extends BaseCommand { }) if (json) { - // TODO(BREAKING_CHANGE): all unwrapping should be removed. - // Users should know based on their arguments if they can expect an array or an object. - // And this unwrapping can break that assumption. - // e.g. `npm view abbrev@^2` should always return an array, but currently since there is only one version matching `^2` this will return a single object instead. + // Users can expect an array . const first = Object.keys(res[0] || {}) const jsonRes = first.length === 1 ? res.map(m => m[first[0]]) : res if (jsonRes.length === 0) { return } - if (jsonRes.length === 1) { - return jsonRes[0] - } return jsonRes } diff --git a/tap-snapshots/test/lib/commands/view.js.test.cjs b/tap-snapshots/test/lib/commands/view.js.test.cjs index 8d66ef780008b..470d4138e90fd 100644 --- a/tap-snapshots/test/lib/commands/view.js.test.cjs +++ b/tap-snapshots/test/lib/commands/view.js.test.cjs @@ -321,22 +321,24 @@ published {TIME} ago ` exports[`test/lib/commands/view.js TAP package with single version full json > must match snapshot 1`] = ` -{ - "_id": "single-version", - "name": "single-version", - "dist-tags": { - "latest": "1.0.0" - }, - "versions": [ - "1.0.0" - ], - "version": "1.0.0", - "dist": { - "shasum": "123", - "tarball": "http://hm.single-version.com/1.0.0.tgz", - "fileCount": 1 +[ + { + "_id": "single-version", + "name": "single-version", + "dist-tags": { + "latest": "1.0.0" + }, + "versions": [ + "1.0.0" + ], + "version": "1.0.0", + "dist": { + "shasum": "123", + "tarball": "http://hm.single-version.com/1.0.0.tgz", + "fileCount": 1 + } } -} +] ` exports[`test/lib/commands/view.js TAP specific field names array field - 1 element > must match snapshot 1`] = ` @@ -415,60 +417,62 @@ error 404 404 exports[`test/lib/commands/view.js TAP workspaces 404 workspaces json > must match snapshot 1`] = ` { - "green": { - "_id": "green", - "name": "green", - "dist-tags": { - "latest": "1.0.0" - }, - "maintainers": [ - { - "name": "claudia", - "email": "c@yellow.com", - "twitter": "cyellow" + "green": [ + { + "_id": "green", + "name": "green", + "dist-tags": { + "latest": "1.0.0" + }, + "maintainers": [ + { + "name": "claudia", + "email": "c@yellow.com", + "twitter": "cyellow" + }, + { + "name": "isaacs", + "email": "i@yellow.com", + "twitter": "iyellow" + } + ], + "keywords": [ + "colors", + "green", + "crayola" + ], + "versions": [ + "1.0.0", + "1.0.1" + ], + "version": "1.0.0", + "description": "green is a very important color", + "bugs": { + "url": "http://bugs.green.com" + }, + "deprecated": true, + "repository": { + "url": "http://repository.green.com" }, - { - "name": "isaacs", - "email": "i@yellow.com", - "twitter": "iyellow" + "license": { + "type": "ACME" + }, + "bin": { + "green": "bin/green.js" + }, + "dependencies": { + "red": "1.0.0", + "yellow": "1.0.0" + }, + "dist": { + "shasum": "123", + "tarball": "http://hm.green.com/1.0.0.tgz", + "integrity": "---", + "fileCount": 1, + "unpackedSize": 1000000000 } - ], - "keywords": [ - "colors", - "green", - "crayola" - ], - "versions": [ - "1.0.0", - "1.0.1" - ], - "version": "1.0.0", - "description": "green is a very important color", - "bugs": { - "url": "http://bugs.green.com" - }, - "deprecated": true, - "repository": { - "url": "http://repository.green.com" - }, - "license": { - "type": "ACME" - }, - "bin": { - "green": "bin/green.js" - }, - "dependencies": { - "red": "1.0.0", - "yellow": "1.0.0" - }, - "dist": { - "shasum": "123", - "tarball": "http://hm.green.com/1.0.0.tgz", - "integrity": "---", - "fileCount": 1, - "unpackedSize": 1000000000 } - }, + ], "error": { "missing-package": { "code": "E404", @@ -530,80 +534,84 @@ error Unknown error exports[`test/lib/commands/view.js TAP workspaces all workspaces --json > must match snapshot 1`] = ` { - "green": { - "_id": "green", - "name": "green", - "dist-tags": { - "latest": "1.0.0" - }, - "maintainers": [ - { - "name": "claudia", - "email": "c@yellow.com", - "twitter": "cyellow" + "green": [ + { + "_id": "green", + "name": "green", + "dist-tags": { + "latest": "1.0.0" + }, + "maintainers": [ + { + "name": "claudia", + "email": "c@yellow.com", + "twitter": "cyellow" + }, + { + "name": "isaacs", + "email": "i@yellow.com", + "twitter": "iyellow" + } + ], + "keywords": [ + "colors", + "green", + "crayola" + ], + "versions": [ + "1.0.0", + "1.0.1" + ], + "version": "1.0.0", + "description": "green is a very important color", + "bugs": { + "url": "http://bugs.green.com" + }, + "deprecated": true, + "repository": { + "url": "http://repository.green.com" + }, + "license": { + "type": "ACME" + }, + "bin": { + "green": "bin/green.js" }, - { - "name": "isaacs", - "email": "i@yellow.com", - "twitter": "iyellow" + "dependencies": { + "red": "1.0.0", + "yellow": "1.0.0" + }, + "dist": { + "shasum": "123", + "tarball": "http://hm.green.com/1.0.0.tgz", + "integrity": "---", + "fileCount": 1, + "unpackedSize": 1000000000 } - ], - "keywords": [ - "colors", - "green", - "crayola" - ], - "versions": [ - "1.0.0", - "1.0.1" - ], - "version": "1.0.0", - "description": "green is a very important color", - "bugs": { - "url": "http://bugs.green.com" - }, - "deprecated": true, - "repository": { - "url": "http://repository.green.com" - }, - "license": { - "type": "ACME" - }, - "bin": { - "green": "bin/green.js" - }, - "dependencies": { - "red": "1.0.0", - "yellow": "1.0.0" - }, - "dist": { - "shasum": "123", - "tarball": "http://hm.green.com/1.0.0.tgz", - "integrity": "---", - "fileCount": 1, - "unpackedSize": 1000000000 } - }, - "orange": { - "name": "orange", - "dist-tags": { - "latest": "1.0.0" - }, - "versions": [ - "1.0.0", - "1.0.1" - ], - "version": "1.0.0", - "homepage": "http://hm.orange.com", - "license": {}, - "dist": { - "shasum": "123", - "tarball": "http://hm.orange.com/1.0.0.tgz", - "integrity": "---", - "fileCount": 1, - "unpackedSize": 1 + ], + "orange": [ + { + "name": "orange", + "dist-tags": { + "latest": "1.0.0" + }, + "versions": [ + "1.0.0", + "1.0.1" + ], + "version": "1.0.0", + "homepage": "http://hm.orange.com", + "license": {}, + "dist": { + "shasum": "123", + "tarball": "http://hm.orange.com/1.0.0.tgz", + "integrity": "---", + "fileCount": 1, + "unpackedSize": 1 + } } - } + ] } ` @@ -658,8 +666,12 @@ orange: exports[`test/lib/commands/view.js TAP workspaces all workspaces single field --json > must match snapshot 1`] = ` { - "green": "green", - "orange": "orange" + "green": [ + "green" + ], + "orange": [ + "orange" + ] } ` @@ -720,59 +732,61 @@ Array [ exports[`test/lib/commands/view.js TAP workspaces single workspace --json > must match snapshot 1`] = ` { - "green": { - "_id": "green", - "name": "green", - "dist-tags": { - "latest": "1.0.0" - }, - "maintainers": [ - { - "name": "claudia", - "email": "c@yellow.com", - "twitter": "cyellow" + "green": [ + { + "_id": "green", + "name": "green", + "dist-tags": { + "latest": "1.0.0" + }, + "maintainers": [ + { + "name": "claudia", + "email": "c@yellow.com", + "twitter": "cyellow" + }, + { + "name": "isaacs", + "email": "i@yellow.com", + "twitter": "iyellow" + } + ], + "keywords": [ + "colors", + "green", + "crayola" + ], + "versions": [ + "1.0.0", + "1.0.1" + ], + "version": "1.0.0", + "description": "green is a very important color", + "bugs": { + "url": "http://bugs.green.com" + }, + "deprecated": true, + "repository": { + "url": "http://repository.green.com" }, - { - "name": "isaacs", - "email": "i@yellow.com", - "twitter": "iyellow" + "license": { + "type": "ACME" + }, + "bin": { + "green": "bin/green.js" + }, + "dependencies": { + "red": "1.0.0", + "yellow": "1.0.0" + }, + "dist": { + "shasum": "123", + "tarball": "http://hm.green.com/1.0.0.tgz", + "integrity": "---", + "fileCount": 1, + "unpackedSize": 1000000000 } - ], - "keywords": [ - "colors", - "green", - "crayola" - ], - "versions": [ - "1.0.0", - "1.0.1" - ], - "version": "1.0.0", - "description": "green is a very important color", - "bugs": { - "url": "http://bugs.green.com" - }, - "deprecated": true, - "repository": { - "url": "http://repository.green.com" - }, - "license": { - "type": "ACME" - }, - "bin": { - "green": "bin/green.js" - }, - "dependencies": { - "red": "1.0.0", - "yellow": "1.0.0" - }, - "dist": { - "shasum": "123", - "tarball": "http://hm.green.com/1.0.0.tgz", - "integrity": "---", - "fileCount": 1, - "unpackedSize": 1000000000 } - } + ] } ` diff --git a/test/lib/commands/view.js b/test/lib/commands/view.js index f1b7b5895113b..84f1edcd96d10 100644 --- a/test/lib/commands/view.js +++ b/test/lib/commands/view.js @@ -465,6 +465,29 @@ t.test('package with --json and semver range', async t => { t.matchSnapshot(joinedOutput()) }) +t.test('package with --json and single-match semver range preserves array output', async t => { + const { view, joinedOutput } = await loadMockNpm(t, { config: { json: true } }) + await view.exec(['single-version@^1']) + const parsed = JSON.parse(joinedOutput()) + t.ok(Array.isArray(parsed), 'preserves the top-level array for semver ranges') + t.equal(parsed.length, 1, 'returns the single matching version in an array') + t.match(parsed[0], { + name: 'single-version', + version: '1.0.0', + dist: { + shasum: '123', + tarball: 'http://hm.single-version.com/1.0.0.tgz', + fileCount: 1, + }, + }, 'returns the expected package data') +}) + +t.test('package field with --json and single-match semver range preserves array output', async t => { + const { view, joinedOutput } = await loadMockNpm(t, { config: { json: true } }) + await view.exec(['single-version@^1', 'version']) + t.strictSame(JSON.parse(joinedOutput()), ['1.0.0'], 'does not unwrap single field matches for semver ranges') +}) + t.test('package with _npmUser.trustedPublisher shows cleaned up property with --json', async t => { const { view, joinedOutput } = await loadMockNpm(t, { config: { json: true } }) await view.exec(['cyan-oidc@^1.0.0']) @@ -480,7 +503,7 @@ t.test('package with --json and no versions', async t => { t.test('package with --json and single string arg', async t => { const { view, joinedOutput } = await loadMockNpm(t, { config: { json: true } }) await view.exec(['blue', 'dist-tags.latest']) - t.equal(JSON.parse(joinedOutput()), '1.0.0', 'no info to display') + t.strictSame(JSON.parse(joinedOutput()), ['1.0.0'], 'returns single string value as array') }) t.test('package with single version', async t => { @@ -494,7 +517,7 @@ t.test('package with single version', async t => { const { view, joinedOutput } = await loadMockNpm(t, { config: { json: true } }) await view.exec(['single-version', 'versions']) const parsed = JSON.parse(joinedOutput()) - t.strictSame(parsed, ['1.0.0'], 'does not unwrap single item arrays in json') + t.strictSame(parsed, [['1.0.0']], 'does not unwrap single item arrays in json') }) t.test('no json and versions arg', async t => { diff --git a/workspaces/arborist/lib/arborist/build-ideal-tree.js b/workspaces/arborist/lib/arborist/build-ideal-tree.js index bc509ca47944c..e91e74d7c0107 100644 --- a/workspaces/arborist/lib/arborist/build-ideal-tree.js +++ b/workspaces/arborist/lib/arborist/build-ideal-tree.js @@ -898,8 +898,21 @@ This is a one-time fix-up, please be patient... // be forced to agree on a version of z. const required = new Set([edge.from]) const parent = edge.peer ? virtualRoot : null - const dep = vrDep && vrDep.satisfies(edge) ? vrDep - : await this.#nodeFromEdge(edge, parent, null, required) + let dep = vrDep && vrDep.satisfies(edge) ? vrDep : null + + // A peerOptional conflict can be resolved by finding an existing node in the tree that satisfies the edge, avoiding a registry fetch that may introduce an extraneous package. See npm/cli#9249. + // Skip the shortcut when the user has signaled an explicit re-fetch intent (npm update by name, explicit request, or audit fix), so we honor those signals rather than silently keeping the existing node. + const skipExistingShortcut = this[_updateNames].includes(edge.name) + || this.#explicitRequests.has(edge) + || (edge.to && this.auditReport?.isVulnerable(edge.to)) + if (!dep && edge.type === 'peerOptional' && !skipExistingShortcut) { + dep = this.#findHoistableNode( + /* istanbul ignore next - resolveParent is always set for non-root nodes */ + edge.from.resolveParent || edge.from, edge) + } + if (!dep) { + dep = await this.#nodeFromEdge(edge, parent, null, required) + } /* istanbul ignore next */ debug(() => { @@ -1029,6 +1042,24 @@ This is a one-time fix-up, please be patient... return this.#buildDepStep() } + // BFS descendants of `root` for a node satisfying `edge`. + // Prefers nodes closer to root. Skips bundled nodes. + #findHoistableNode (root, edge) { + const queue = [...root.children.values()] + while (queue.length) { + const node = queue.shift() + if (node.name === edge.name + && !node.inDepBundle + && node.satisfies(edge)) { + return node + } + for (const child of node.children.values()) { + queue.push(child) + } + } + return null + } + // loads a node from an edge, and then loads its peer deps (and their peer deps, on down the line) into a virtual root parent. async #nodeFromEdge (edge, parent_, secondEdge, required) { // create a virtual root node with the same deps as the node that is requesting this one, so that we can get all the peer deps in a context where they're likely to be resolvable. diff --git a/workspaces/arborist/test/arborist/build-ideal-tree.js b/workspaces/arborist/test/arborist/build-ideal-tree.js index d1e1895baa6ee..270040425d58a 100644 --- a/workspaces/arborist/test/arborist/build-ideal-tree.js +++ b/workspaces/arborist/test/arborist/build-ideal-tree.js @@ -4482,6 +4482,140 @@ t.test('skip invalid peerOptional edges in problemEdges when save=false (#8726)' t.ok(tree.children.get('util'), 'util is in the tree') }) +t.test('peerOptional prefers existing tree node over registry fetch (#9249)', async t => { + // Reproduction: ts-jest has peerOptional jest-util@"^29||^30". + // @types/jest@28 → expect@28 → jest-util@28 placed at root first. + // jest@29 → jest-util@29 nested (root slot taken by @28). + // ts-jest re-queued, peerOptional jest-util resolves to root @28 → INVALID. + // Without fix: #nodeFromEdge fetches jest-util@30 (latest ^29||^30), blocks @29. + // With fix: #findHoistableNode finds nested @29, PlaceDep hoists it to root. + const registry = createRegistry(t, false) + + const jestPack = registry.packument({ + name: 'jest', + version: '29.0.0', + dependencies: { 'jest-util': '^29.0.0' }, + }) + const jestManifest = registry.manifest({ name: 'jest', packuments: [jestPack] }) + await registry.package({ manifest: jestManifest }) + + const tsJestPack = registry.packument({ + name: 'ts-jest', + version: '29.0.0', + peerDependencies: { jest: '^29.0.0', 'jest-util': '^29.0.0 || ^30.0.0' }, + peerDependenciesMeta: { 'jest-util': { optional: true } }, + }) + const tsJestManifest = registry.manifest({ name: 'ts-jest', packuments: [tsJestPack] }) + await registry.package({ manifest: tsJestManifest }) + + const expectPack = registry.packument({ + name: 'expect', + version: '28.0.0', + dependencies: { 'jest-util': '^28.0.0' }, + }) + const expectManifest = registry.manifest({ name: 'expect', packuments: [expectPack] }) + await registry.package({ manifest: expectManifest }) + + const atTypesPack = registry.packument({ + name: '@types/jest', + version: '28.0.0', + dependencies: { expect: '^28.0.0' }, + }) + const atTypesManifest = registry.manifest({ name: '@types/jest', packuments: [atTypesPack] }) + await registry.package({ manifest: atTypesManifest }) + + // Only publish 28, 29, and 30. + const jestUtilPacks = registry.packuments(['28.0.0', '29.0.0', '30.0.0'], 'jest-util') + const jestUtilManifest = registry.manifest({ name: 'jest-util', packuments: jestUtilPacks }) + await registry.package({ manifest: jestUtilManifest, times: 3 }) + + const path = t.testdir({ + 'package.json': JSON.stringify({ + dependencies: { + jest: '^29.0.0', + 'ts-jest': '^29.0.0', + '@types/jest': '^28.0.0', + }, + }), + }) + + const arb = newArb(path) + const tree = await arb.buildIdealTree() + + // jest-util@29 at root — found via #findHoistableNode, not fetched as @30 + t.equal(tree.children.get('jest-util').version, '29.0.0', + 'jest-util@29 hoisted to root from nested location') + + // ts-jest's peerOptional resolved to @29 from the tree, not @30 from registry + const tsJest = tree.children.get('ts-jest') + const peerOptEdge = tsJest.edgesOut.get('jest-util') + t.equal(peerOptEdge.to.version, '29.0.0', + 'ts-jest peerOptional jest-util resolved to @29') + + // jest-util@28 nested under expect (incompatible with root @29) + const expectNode = [...tree.inventory.query('name', 'expect')][0] + t.equal(expectNode?.children?.get('jest-util')?.version, '28.0.0', + 'jest-util@28 nested under expect') +}) + +t.test('peerOptional skips dedupe shortcut when update.names includes the dep', async t => { + // Same scenario as above, but with update: { names: ['jest-util'] }. + // skipExistingShortcut=true so #findHoistableNode is NOT called; + // #nodeFromEdge fetches from registry, getting jest-util@30 (latest matching ^29||^30). + const registry = createRegistry(t, false) + + const jestPack = registry.packument({ + name: 'jest', + version: '29.0.0', + dependencies: { 'jest-util': '^29.0.0' }, + }) + await registry.package({ manifest: registry.manifest({ name: 'jest', packuments: [jestPack] }) }) + + const tsJestPack = registry.packument({ + name: 'ts-jest', + version: '29.0.0', + peerDependencies: { jest: '^29.0.0', 'jest-util': '^29.0.0 || ^30.0.0' }, + peerDependenciesMeta: { 'jest-util': { optional: true } }, + }) + await registry.package({ manifest: registry.manifest({ name: 'ts-jest', packuments: [tsJestPack] }) }) + + const expectPack = registry.packument({ + name: 'expect', + version: '28.0.0', + dependencies: { 'jest-util': '^28.0.0' }, + }) + await registry.package({ manifest: registry.manifest({ name: 'expect', packuments: [expectPack] }) }) + + const atTypesPack = registry.packument({ + name: '@types/jest', + version: '28.0.0', + dependencies: { expect: '^28.0.0' }, + }) + await registry.package({ manifest: registry.manifest({ name: '@types/jest', packuments: [atTypesPack] }) }) + + const jestUtilPacks = registry.packuments(['28.0.0', '29.0.0', '30.0.0'], 'jest-util') + await registry.package({ manifest: registry.manifest({ name: 'jest-util', packuments: jestUtilPacks }), times: 3 }) + + const path = t.testdir({ + 'package.json': JSON.stringify({ + dependencies: { + jest: '^29.0.0', + 'ts-jest': '^29.0.0', + '@types/jest': '^28.0.0', + }, + }), + }) + + const arb = newArb(path) + const tree = await arb.buildIdealTree({ update: { names: ['jest-util'] } }) + + // With skipExistingShortcut=true, #nodeFromEdge fetches from registry + // so jest-util@30 (latest matching ^29||^30) is used instead of deduping @29 + const tsJest = tree.children.get('ts-jest') + const peerOptEdge = tsJest.edgesOut.get('jest-util') + t.equal(peerOptEdge.to?.version, '30.0.0', 'peerOptional jest-util refetched to @30, not deduped to @29') +}) + t.test('overrides with bundledDependencies', async t => { t.test('does not infinite loop with bundledDependencies and overrides', async t => { // https://github.com/npm/cli/issues/9227