From d9238af0b4a3d368b79e54d1636e4aab80bb537f Mon Sep 17 00:00:00 2001 From: isaacs Date: Fri, 28 Jun 2019 15:08:02 -0700 Subject: [PATCH] fix: do not crash when removing nameless packages Fix: npm/npm#17858 Fix: npm/npm#18042 Fix: https://npm.community/t/issue-npm-dedupe-crash-with-typeerror-cannot-read-property-0-of-undefined/644/3 Close: https://github.com/npm/cli/pull/201 This fixes a bug where a package folder might have a package.json which is missing or lacks a name property. It also properly detects the scoped-ness of a package folder even if the package name is not scoped, since one might install `express@npm:@scope/express` and end up in that state. --- lib/unbuild.js | 4 +++- test/tap/uninstall-package.js | 24 +++++++++++++++++++++++- 2 files changed, 26 insertions(+), 2 deletions(-) diff --git a/lib/unbuild.js b/lib/unbuild.js index d527778e92b0..3e115b6999b2 100644 --- a/lib/unbuild.js +++ b/lib/unbuild.js @@ -58,7 +58,9 @@ function rmStuff (pkg, folder, cb) { // if it's global, and folder is in {prefix}/node_modules, // then bins are in {prefix}/bin // otherwise, then bins are in folder/../.bin - var parent = pkg.name[0] === '@' ? path.dirname(path.dirname(folder)) : path.dirname(folder) + var dir = path.dirname(folder) + var scope = path.basename(dir) + var parent = scope.charAt(0) === '@' ? path.dirname(dir) : dir var gnm = npm.dir // gnm might be an absolute path, parent might be relative // this checks they're the same directory regardless diff --git a/test/tap/uninstall-package.js b/test/tap/uninstall-package.js index 56df2e17e41c..87fdee228169 100644 --- a/test/tap/uninstall-package.js +++ b/test/tap/uninstall-package.js @@ -18,7 +18,8 @@ var json = { version: '0.0.0', dependencies: { underscore: '~1.3.1', - request: '~0.9.0' + request: '~0.9.0', + '@isaacs/namespace-test': '1.x' } } @@ -69,6 +70,27 @@ test('returns a list of removed items', function (t) { }) }) +test('does not fail if installed package lacks a name somehow', function (t) { + const scope = path.resolve(pkg, 'node_modules/@isaacs') + const scopePkg = path.resolve(scope, 'namespace-test') + const pj = path.resolve(scopePkg, 'package.json') + fs.writeFileSync(pj, JSON.stringify({ + lol: 'yolo', + name: 99 + })) + common.npm( + ['uninstall', '@isaacs/namespace-test'], + EXEC_OPTS, + function (err, code, stdout, stderr) { + if (err) throw err + t.equal(code, 0, 'should exit successfully') + t.has(stdout, /removed 1 package in/) + t.notOk(fs.existsSync(scope), 'scoped package removed') + t.end() + } + ) +}) + test('cleanup', function (t) { cleanup() t.end()