From a6546293ca5eea04c6973b36db0312a59e8cbf84 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kat=20March=C3=A1n?= Date: Wed, 11 Oct 2017 17:03:58 -0700 Subject: [PATCH] feat(optional): ignore failed optional deps (#27) Fixes: #3 --- index.js | 59 ++++++++++++++++++++++++ package-lock.json | 6 +-- package.json | 2 +- test/specs/index.js | 108 ++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 171 insertions(+), 4 deletions(-) diff --git a/index.js b/index.js index bfcbd4c..e705655 100644 --- a/index.js +++ b/index.js @@ -29,12 +29,16 @@ class Installer { // Misc this.log = npmlog this.pkg = null + this.tree = null + this.failedDeps = new Set() + this.purgedDeps = new Set() } run () { return this.prepare() .then(() => this.runScript('preinstall', this.pkg, this.prefix)) .then(() => this.extractTree(this.logicalTree)) + .then(() => this.garbageCollect(this.logicalTree)) .then(() => this.runScript('install', this.pkg, this.prefix)) .then(() => this.runScript('postinstall', this.pkg, this.prefix)) .then(() => this.runScript('prepublish', this.pkg, this.prefix)) @@ -137,11 +141,64 @@ class Installer { this.pkgCount++ return this }) + .catch(e => { + if (child.optional) { + this.pkgCount++ + this.failedDeps.add(child) + return rimraf(childPath) + } else { + throw e + } + }) }) return child.pending }, { concurrency: 50 }) } + // A cute little mark-and-sweep collector! + garbageCollect (tree) { + if (!this.failedDeps.size) { return } + + const liveDeps = new Set() + const installer = this + const seen = new Set() + const failed = this.failedDeps + const purged = this.purgedDeps + mark(tree) + seen.clear() + return sweep(tree) + + function mark (tree) { + for (let dep of tree.dependencies.values()) { + if (seen.has(dep)) { continue } + seen.add(dep) + if (!failed.has(dep)) { + liveDeps.add(dep) + mark(dep) + } + } + } + + function sweep (tree) { + return BB.map(tree.dependencies.values(), dep => { + if (seen.has(dep)) { return } + seen.add(dep) + return sweep(dep).then(() => { + if (!liveDeps.has(dep) && !purged.has(dep)) { + const depPath = path.join( + installer.prefix, + 'node_modules', + dep.address.replace(/:/g, '/node_modules/') + ) + installer.pkgCount-- + purged.add(dep) + return rimraf(depPath) + } + }) + }, { concurrency: 100 }) + } + } + runScript (stage, pkg, pkgPath) { if (!this.config.lifecycleOpts.ignoreScripts && pkg.scripts && pkg.scripts[stage]) { // TODO(mikesherov): remove pkg._id when npm-lifecycle no longer relies on it @@ -151,7 +208,9 @@ class Installer { return BB.resolve() } } + module.exports = Installer + module.exports._readJson = readJson function readJson (jsonPath, name, ignoreMissing) { diff --git a/package-lock.json b/package-lock.json index 200d7b3..a22abe9 100644 --- a/package-lock.json +++ b/package-lock.json @@ -2962,9 +2962,9 @@ } }, "npm-logical-tree": { - "version": "1.0.0", - "resolved": "https://registry.npmjs.org/npm-logical-tree/-/npm-logical-tree-1.0.0.tgz", - "integrity": "sha512-BoF9a9GNQgTJ6xbovo3wBmmVkDRdzqOkWfdUn5PDTdkiYcB4GXBk5inN/csKQMcXd4Z8zZs6Lqke6J+LK1iFCQ==" + "version": "1.1.0", + "resolved": "https://registry.npmjs.org/npm-logical-tree/-/npm-logical-tree-1.1.0.tgz", + "integrity": "sha512-xAzsBAUPJPQFbFJ8Ajz8tSXe6w+WZRPNZUCP/pTYvLJa5VTTcAE3jqczQADWMjowGKhSPZIp+W8lF6UN9va4rQ==" }, "npm-package-arg": { "version": "5.1.2", diff --git a/package.json b/package.json index d05c60e..217e371 100644 --- a/package.json +++ b/package.json @@ -52,7 +52,7 @@ "graceful-fs": "^4.1.11", "lock-verify": "^1.1.0", "npm-lifecycle": "^1.0.3", - "npm-logical-tree": "^1.0.0", + "npm-logical-tree": "^1.1.0", "npm-package-arg": "^5.1.2", "npmlog": "^4.1.2", "pacote": "^6.0.4", diff --git a/test/specs/index.js b/test/specs/index.js index 83047a0..7daedc4 100644 --- a/test/specs/index.js +++ b/test/specs/index.js @@ -120,6 +120,8 @@ test('handles dependency list with only shallow subdeps', t => { } }), 'package-lock.json': File({ + name: pkgName, + verson: pkgVersion, dependencies: { a: { version: '1.1.1' @@ -262,6 +264,112 @@ test('prioritizes npm-shrinkwrap over package-lock if both present', t => { }) }) +test('removes failed optional dependencies', t => { + const fixture = new Tacks(Dir({ + 'package.json': File({ + name: pkgName, + version: pkgVersion, + dependencies: { + a: '^1' + }, + optionalDependencies: { + b: '^2' + } + }), + 'package-lock.json': File({ + lockfileVersion: 1, + requires: true, + dependencies: { + a: { + version: '1.0.0', + requires: { + b: '2.0.0', + d: '4.0.0' + } + }, + b: { + version: '2.0.0', + optional: true, + requires: { + c: '3.0.0', + d: '4.0.0' + } + }, + c: { + version: '3.0.0', + optional: true + }, + d: { + version: '4.0.0' + } + } + }) + })) + fixture.create(prefix) + + extract = (name, child, childPath, opts) => { + let files + if (child.name === 'a') { + files = new Tacks(Dir({ + 'package.json': File({ + name: 'a', + version: '1.0.0', + dependencies: { + b: '^2', + d: '^4' + } + }) + })) + } else if (child.name === 'b') { + files = new Tacks(Dir({ + 'package.json': File({ + name: 'b', + version: '2.0.0', + dependencies: { + c: '^3', + d: '^4' + }, + scripts: { + install: 'exit 1' + } + }) + })) + } else if (child.name === 'c') { + files = new Tacks(Dir({ + 'package.json': File({ + name: 'c', + version: '3.0.0' + }) + })) + } else if (child.name === 'd') { + files = new Tacks(Dir({ + 'package.json': File({ + name: 'd', + version: '4.0.0' + }) + })) + } + files.create(childPath) + } + + const originalConsoleLog = console.log + console.log = () => {} + return new Installer({prefix}).run().then(details => { + console.log = originalConsoleLog + t.ok(true, 'installer succeeded even with optDep failure') + t.equal(details.pkgCount, 2, 'only successful deps counted') + const modP = path.join(prefix, 'node_modules') + t.ok(fs.statSync(path.join(modP, 'a')), 'dep a is there') + t.ok(fs.statSync(path.join(modP, 'd')), 'transitive dep d is there') + t.throws(() => { + fs.statSync(path.join(prefix, 'node_modules', 'b')) + }, 'failed optional dep b not in node_modules') + t.throws(() => { + fs.statSync(path.join(prefix, 'node_modules', 'c')) + }, 'isolated dependency d of failed dep removed') + }) +}) + test('runs lifecycle hooks of packages with env variables', t => { const originalConsoleLog = console.log console.log = () => {}