From cb6eb0d206b7e2f63d5c7a7a17bea4aed1b9f2bf Mon Sep 17 00:00:00 2001 From: isaacs Date: Thu, 1 Apr 2021 11:50:59 -0700 Subject: [PATCH] Support reporting on ERESOLVE errors when current is missing While it is somewhat helpful to get reports on this, since it indicates an underlying problem in Arborist, it's also very disruptive and unhelpful for users. As of 2.3.0, Arborist gives us the currentEdge if available, so we _can_ report on that at least. If there is no node _or_ edge, then we just don't say what the current state is, which isn't useful, but at least is less annoying than a 'property of null' exception. PR-URL: https://github.com/npm/cli/pull/3015 Credit: @isaacs Close: #3015 Reviewed-by: @wraithgar --- lib/utils/explain-eresolve.js | 11 +- ...-lib-utils-explain-eresolve.js-TAP.test.js | 182 +++++++++++++++ test/fixtures/eresolve-explanations.js | 209 ++++++++++++------ 3 files changed, 333 insertions(+), 69 deletions(-) diff --git a/lib/utils/explain-eresolve.js b/lib/utils/explain-eresolve.js index 69789ec9a1c2d..cda77aff94113 100644 --- a/lib/utils/explain-eresolve.js +++ b/lib/utils/explain-eresolve.js @@ -15,13 +15,20 @@ const { explainEdge, explainNode, printNode } = require('./explain-dep.js') // The full report (ie, depth=Infinity) is always written to the cache folder // at ${cache}/eresolve-report.txt along with full json. const explainEresolve = (expl, color, depth) => { - const { edge, current, peerConflict } = expl + const { edge, current, peerConflict, currentEdge } = expl const out = [] if (edge.from && edge.from.whileInstalling) out.push('While resolving: ' + printNode(edge.from.whileInstalling, color)) - out.push('Found: ' + explainNode(current, depth, color)) + // it "should" be impossible for an ERESOLVE explanation to lack both + // current and currentEdge, but better to have a less helpful error + // than a crashing failure. + if (current) + out.push('Found: ' + explainNode(current, depth, color)) + else if (currentEdge) + out.push('Found: ' + explainEdge(currentEdge, depth, color)) + out.push('\nCould not resolve dependency:\n' + explainEdge(edge, depth, color)) diff --git a/tap-snapshots/test-lib-utils-explain-eresolve.js-TAP.test.js b/tap-snapshots/test-lib-utils-explain-eresolve.js-TAP.test.js index 87dcb861c633f..e066680153021 100644 --- a/tap-snapshots/test-lib-utils-explain-eresolve.js-TAP.test.js +++ b/tap-snapshots/test-lib-utils-explain-eresolve.js-TAP.test.js @@ -418,6 +418,188 @@ to accept an incorrect (and potentially broken) dependency resolution. See \${REPORT} for a full report. ` +exports[`test/lib/utils/explain-eresolve.js TAP no current node, but has current edge > explain with color 1`] = ` +While resolving: eslint@7.22.0 +Found: dev eslint@"file:." from the root project + +Could not resolve dependency: +peer eslint@"^6.0.0" from eslint-plugin-jsdoc@22.2.0 +node_modules/eslint-plugin-jsdoc + dev eslint-plugin-jsdoc@"^22.1.0" from the root project +` + +exports[`test/lib/utils/explain-eresolve.js TAP no current node, but has current edge > explain with no color, depth of 6 1`] = ` +While resolving: eslint@7.22.0 +Found: dev eslint@"file:." from the root project + +Could not resolve dependency: +peer eslint@"^6.0.0" from eslint-plugin-jsdoc@22.2.0 +node_modules/eslint-plugin-jsdoc + dev eslint-plugin-jsdoc@"^22.1.0" from the root project +` + +exports[`test/lib/utils/explain-eresolve.js TAP no current node, but has current edge > report 1`] = ` +# npm resolution error report + +\${TIME} + +While resolving: eslint@7.22.0 +Found: dev eslint@"file:." from the root project + +Could not resolve dependency: +peer eslint@"^6.0.0" from eslint-plugin-jsdoc@22.2.0 +node_modules/eslint-plugin-jsdoc + dev eslint-plugin-jsdoc@"^22.1.0" from the root project + +Fix the upstream dependency conflict, or retry +this command with --force, or --legacy-peer-deps +to accept an incorrect (and potentially broken) dependency resolution. + +Raw JSON explanation object: + +{ + "name": "no current node, but has current edge", + "json": true +} + +` + +exports[`test/lib/utils/explain-eresolve.js TAP no current node, but has current edge > report with color 1`] = ` +While resolving: eslint@7.22.0 +Found: dev eslint@"file:." from the root project + +Could not resolve dependency: +peer eslint@"^6.0.0" from eslint-plugin-jsdoc@22.2.0 +node_modules/eslint-plugin-jsdoc + dev eslint-plugin-jsdoc@"^22.1.0" from the root project + +Fix the upstream dependency conflict, or retry +this command with --force, or --legacy-peer-deps +to accept an incorrect (and potentially broken) dependency resolution. + +See \${REPORT} for a full report. +` + +exports[`test/lib/utils/explain-eresolve.js TAP no current node, but has current edge > report with color, depth only 2 1`] = ` +While resolving: eslint@7.22.0 +Found: dev eslint@"file:." from the root project + +Could not resolve dependency: +peer eslint@"^6.0.0" from eslint-plugin-jsdoc@22.2.0 +node_modules/eslint-plugin-jsdoc + dev eslint-plugin-jsdoc@"^22.1.0" from the root project + +Fix the upstream dependency conflict, or retry +this command with --force, or --legacy-peer-deps +to accept an incorrect (and potentially broken) dependency resolution. + +See \${REPORT} for a full report. +` + +exports[`test/lib/utils/explain-eresolve.js TAP no current node, but has current edge > report with no color, depth of 6 1`] = ` +While resolving: eslint@7.22.0 +Found: dev eslint@"file:." from the root project + +Could not resolve dependency: +peer eslint@"^6.0.0" from eslint-plugin-jsdoc@22.2.0 +node_modules/eslint-plugin-jsdoc + dev eslint-plugin-jsdoc@"^22.1.0" from the root project + +Fix the upstream dependency conflict, or retry +this command with --force, or --legacy-peer-deps +to accept an incorrect (and potentially broken) dependency resolution. + +See \${REPORT} for a full report. +` + +exports[`test/lib/utils/explain-eresolve.js TAP no current node, no current edge, idk > explain with color 1`] = ` +While resolving: eslint@7.22.0 + +Could not resolve dependency: +peer eslint@"^6.0.0" from eslint-plugin-jsdoc@22.2.0 +node_modules/eslint-plugin-jsdoc + dev eslint-plugin-jsdoc@"^22.1.0" from the root project +` + +exports[`test/lib/utils/explain-eresolve.js TAP no current node, no current edge, idk > explain with no color, depth of 6 1`] = ` +While resolving: eslint@7.22.0 + +Could not resolve dependency: +peer eslint@"^6.0.0" from eslint-plugin-jsdoc@22.2.0 +node_modules/eslint-plugin-jsdoc + dev eslint-plugin-jsdoc@"^22.1.0" from the root project +` + +exports[`test/lib/utils/explain-eresolve.js TAP no current node, no current edge, idk > report 1`] = ` +# npm resolution error report + +\${TIME} + +While resolving: eslint@7.22.0 + +Could not resolve dependency: +peer eslint@"^6.0.0" from eslint-plugin-jsdoc@22.2.0 +node_modules/eslint-plugin-jsdoc + dev eslint-plugin-jsdoc@"^22.1.0" from the root project + +Fix the upstream dependency conflict, or retry +this command with --force, or --legacy-peer-deps +to accept an incorrect (and potentially broken) dependency resolution. + +Raw JSON explanation object: + +{ + "name": "no current node, no current edge, idk", + "json": true +} + +` + +exports[`test/lib/utils/explain-eresolve.js TAP no current node, no current edge, idk > report with color 1`] = ` +While resolving: eslint@7.22.0 + +Could not resolve dependency: +peer eslint@"^6.0.0" from eslint-plugin-jsdoc@22.2.0 +node_modules/eslint-plugin-jsdoc + dev eslint-plugin-jsdoc@"^22.1.0" from the root project + +Fix the upstream dependency conflict, or retry +this command with --force, or --legacy-peer-deps +to accept an incorrect (and potentially broken) dependency resolution. + +See \${REPORT} for a full report. +` + +exports[`test/lib/utils/explain-eresolve.js TAP no current node, no current edge, idk > report with color, depth only 2 1`] = ` +While resolving: eslint@7.22.0 + +Could not resolve dependency: +peer eslint@"^6.0.0" from eslint-plugin-jsdoc@22.2.0 +node_modules/eslint-plugin-jsdoc + dev eslint-plugin-jsdoc@"^22.1.0" from the root project + +Fix the upstream dependency conflict, or retry +this command with --force, or --legacy-peer-deps +to accept an incorrect (and potentially broken) dependency resolution. + +See \${REPORT} for a full report. +` + +exports[`test/lib/utils/explain-eresolve.js TAP no current node, no current edge, idk > report with no color, depth of 6 1`] = ` +While resolving: eslint@7.22.0 + +Could not resolve dependency: +peer eslint@"^6.0.0" from eslint-plugin-jsdoc@22.2.0 +node_modules/eslint-plugin-jsdoc + dev eslint-plugin-jsdoc@"^22.1.0" from the root project + +Fix the upstream dependency conflict, or retry +this command with --force, or --legacy-peer-deps +to accept an incorrect (and potentially broken) dependency resolution. + +See \${REPORT} for a full report. +` + exports[`test/lib/utils/explain-eresolve.js TAP withShrinkwrap > explain with color 1`] = ` While resolving: @isaacs/peer-dep-cycle-b@1.0.0 Found: @isaacs/peer-dep-cycle-c@2.0.0 diff --git a/test/fixtures/eresolve-explanations.js b/test/fixtures/eresolve-explanations.js index c5c338e61c8c0..b6ccac7d3d627 100644 --- a/test/fixtures/eresolve-explanations.js +++ b/test/fixtures/eresolve-explanations.js @@ -16,10 +16,10 @@ module.exports = { type: 'prod', name: '@isaacs/peer-dep-cycle-a', spec: '1.x', - from: { location: '/some/project' } - } - ] - } + from: { location: '/some/project' }, + }, + ], + }, }, current: { name: '@isaacs/peer-dep-cycle-c', @@ -30,9 +30,9 @@ module.exports = { type: 'prod', name: '@isaacs/peer-dep-cycle-c', spec: '2.x', - from: { location: '/some/project' } - } - ] + from: { location: '/some/project' }, + }, + ], }, peerConflict: { name: '@isaacs/peer-dep-cycle-c', @@ -63,15 +63,15 @@ module.exports = { type: 'prod', name: '@isaacs/peer-dep-cycle-a', spec: '1.x', - from: { location: '/some/project' } - } - ] - } - } - ] - } - } - ] + from: { location: '/some/project' }, + }, + ], + }, + }, + ], + }, + }, + ], }, strictPeerDeps: true, }, @@ -102,13 +102,13 @@ module.exports = { type: 'prod', name: '@isaacs/peer-dep-cycle-a', spec: '1.x', - from: { location: '/some/project' } - } - ] - } - } - ] - } + from: { location: '/some/project' }, + }, + ], + }, + }, + ], + }, }, current: { name: '@isaacs/peer-dep-cycle-c', @@ -119,9 +119,9 @@ module.exports = { type: 'prod', name: '@isaacs/peer-dep-cycle-c', spec: '2.x', - from: { location: '/some/project' } - } - ] + from: { location: '/some/project' }, + }, + ], }, strictPeerDeps: true, }, @@ -134,7 +134,7 @@ module.exports = { whileInstalling: { name: 'project', version: '1.2.3', - path: '/some/project' + path: '/some/project', }, location: 'node_modules/@isaacs/testing-peer-dep-conflict-chain-d', dependents: [ @@ -142,9 +142,9 @@ module.exports = { type: 'prod', name: '@isaacs/testing-peer-dep-conflict-chain-d', spec: '2', - from: { location: '/some/project' } - } - ] + from: { location: '/some/project' }, + }, + ], }, edge: { type: 'peer', @@ -157,7 +157,7 @@ module.exports = { whileInstalling: { name: 'project', version: '1.2.3', - path: '/some/project' + path: '/some/project', }, location: 'node_modules/@isaacs/testing-peer-dep-conflict-chain-c', dependents: [ @@ -165,13 +165,13 @@ module.exports = { type: 'prod', name: '@isaacs/testing-peer-dep-conflict-chain-c', spec: '1', - from: { location: '/some/project' } - } - ] - } + from: { location: '/some/project' }, + }, + ], + }, }, peerConflict: null, - strictPeerDeps: false + strictPeerDeps: false, }, gatsby: { @@ -182,7 +182,7 @@ module.exports = { whileInstalling: { name: 'gatsby-recipes', version: '0.2.31', - path: '/some/project/node_modules/gatsby-recipes' + path: '/some/project/node_modules/gatsby-recipes', }, location: 'node_modules/ink', dependents: [ @@ -218,19 +218,19 @@ module.exports = { name: 'gatsby', spec: '', from: { - location: '/some/project/gatsby-user' - } - } - ] - } - } - ] - } - } - ] - } - } - ] + location: '/some/project/gatsby-user', + }, + }, + ], + }, + }, + ], + }, + }, + ], + }, + }, + ], }, edge: { type: 'peer', @@ -243,7 +243,7 @@ module.exports = { whileInstalling: { name: 'gatsby-recipes', version: '0.2.31', - path: '/some/project/gatsby-user/node_modules/gatsby-recipes' + path: '/some/project/gatsby-user/node_modules/gatsby-recipes', }, location: 'node_modules/ink-box', dependents: [ @@ -279,23 +279,98 @@ module.exports = { name: 'gatsby', spec: '', from: { - location: '/some/project/gatsby-user' - } - } - ] - } - } - ] - } - } - ] - } - } - ] - } + location: '/some/project/gatsby-user', + }, + }, + ], + }, + }, + ], + }, + }, + ], + }, + }, + ], + }, }, peerConflict: null, - strictPeerDeps: true - } + strictPeerDeps: true, + }, + 'no current node, but has current edge': { + code: 'ERESOLVE', + current: null, + currentEdge: { + type: 'dev', + name: 'eslint', + spec: 'file:.', + error: 'MISSING', + from: { + location: '/some/projects/eslint', + }, + }, + edge: { + type: 'peer', + name: 'eslint', + spec: '^6.0.0', + error: 'MISSING', + from: { + name: 'eslint-plugin-jsdoc', + version: '22.2.0', + whileInstalling: { + name: 'eslint', + version: '7.22.0', + path: '/Users/isaacs/dev/npm/cli/eslint', + }, + location: 'node_modules/eslint-plugin-jsdoc', + dependents: [ + { + type: 'dev', + name: 'eslint-plugin-jsdoc', + spec: '^22.1.0', + from: { + location: '/some/projects/eslint', + }, + }, + ], + }, + }, + peerConflict: null, + strictPeerDeps: false, + force: false, + }, + 'no current node, no current edge, idk': { + code: 'ERESOLVE', + current: null, + edge: { + type: 'peer', + name: 'eslint', + spec: '^6.0.0', + error: 'MISSING', + from: { + name: 'eslint-plugin-jsdoc', + version: '22.2.0', + whileInstalling: { + name: 'eslint', + version: '7.22.0', + path: '/Users/isaacs/dev/npm/cli/eslint', + }, + location: 'node_modules/eslint-plugin-jsdoc', + dependents: [ + { + type: 'dev', + name: 'eslint-plugin-jsdoc', + spec: '^22.1.0', + from: { + location: '/some/projects/eslint', + }, + }, + ], + }, + }, + peerConflict: null, + strictPeerDeps: false, + force: false, + }, }