From d6b2d6ff592d0539c2719214ff827d0ac2b1cc4d Mon Sep 17 00:00:00 2001 From: Nils Knappmeier Date: Wed, 11 Apr 2018 00:05:21 +0200 Subject: [PATCH] feat: support for missing modules with existing dependencies closes #8 --- .thought/partials/howitworks.md.hbs | 42 ++++++++++++ README.md | 59 +++++++++++++++- src/DependencyTree.js | 20 ++++-- src/Package.js | 47 +++++++++---- src/PackageStats.js | 4 +- src/index.js | 11 ++- test/Package-spec.js | 68 +++++++++++++++++-- .../node_modules/dep2/package.json | 6 ++ .../node_modules/devdep1/package.json | 6 ++ test/fixtures/moduleWithMissingDependent.txt | 7 ++ .../moduleWithMissingDependent/index.js | 0 .../node_modules/dep1/package.json | 6 ++ .../node_modules/dep2/package.json | 6 ++ .../moduleWithMissingDependent/package.json | 6 ++ test/index-spec.js | 9 +++ 15 files changed, 266 insertions(+), 31 deletions(-) create mode 100644 .thought/partials/howitworks.md.hbs create mode 100644 test/fixtures/moduleWithCyclicDeps/node_modules/dep2/package.json create mode 100644 test/fixtures/moduleWithCyclicDeps/node_modules/devdep1/package.json create mode 100644 test/fixtures/moduleWithMissingDependent.txt create mode 100644 test/fixtures/moduleWithMissingDependent/index.js create mode 100644 test/fixtures/moduleWithMissingDependent/node_modules/dep1/package.json create mode 100644 test/fixtures/moduleWithMissingDependent/node_modules/dep2/package.json create mode 100644 test/fixtures/moduleWithMissingDependent/package.json diff --git a/.thought/partials/howitworks.md.hbs b/.thought/partials/howitworks.md.hbs new file mode 100644 index 0000000..a94643a --- /dev/null +++ b/.thought/partials/howitworks.md.hbs @@ -0,0 +1,42 @@ +# How it works + +`{{packageJson.name}}` collects all modules from the `node_modules` directory, the `node_modules` directory of +each of those modules and the `node_modules` directory in that modules, and so on. + +When all packages have been collected, it reads the package.json of each module and uses the `_location`-property +and the `_requiredBy`-property to recreate the complete dependency tree. + +* `_location` contains the location of the module in the directory tree. A module in `node_modules/packageA/node_modules/packageB` + has the location `/packageA/packageB` +* `_requiredBy` contains a list of module that are dependent on the current module. For each such module, it contains + the value of the `_location`-property. + +Once the packages is connected, the stats for each package are computed: + +* The number of dependencies is computed transitively across the tree. +* The total kilobytes (1024 bytes) is computed, include all dependencies. + The computation of file sizes assumes that only whole blocks are used, even by small files. The `blksize`-property + of the [fs.Stats-object]() is used as block size. If this value is missing (e.g. on Windows), a size of 4096 is + used. + +## Caveats + +In some cases, the dependencies in the `node_modules`-directory are tempered with. For example, {{npm 'lerna'}} +combines dependencies of multiple packages in the `node_modules`-directory of the root-project and removes +obsolete dependencies from the tree. This can lead to cycles in the dependency tree which are displayed in +the output like this: + +{{include 'test/fixtures/moduleWithCyclicDeps.txt'}} + +Furthermore, this and the use of optional dependencies can lead to a situation where a package is `_requiredBy` +an existing dependency (i.e. a dependent package) but does not exist anymore in the tree. For those delete packages, +a dummy package is displayed in a separate tree. + +{{include 'test/fixtures/moduleWithMissingDependent.txt'}} + +In this example, a module `dep2@1.0.0` was found. The `_requireBy`-property shows that `dep2` +is part of the tree, because it is a dependency of a module that should be in `node_modules/dep3`, +which could not be found. + + + diff --git a/README.md b/README.md index 48fceb2..5bec8ce 100644 --- a/README.md +++ b/README.md @@ -29,7 +29,7 @@ Run `analyze-module-size` in your project directory. The output will be somethin (Note that the displayed sizes are accumulated from the each module an its dependencies): ``` -size: 64k... with-dependencies: 1200k +size: 68k... with-dependencies: 1204k ├─┬ globby@6.1.0, 488k, 17 deps │ ├─┬ glob@7.1.2, 344k, 10 deps │ │ ├─┬ minimatch@3.0.4, 136k, 3 deps @@ -101,6 +101,63 @@ Usage: analyze-module-size [options] ``` +# How it works + +`` collects all modules from the `node_modules` directory, the `node_modules` directory of +each of those modules and the `node_modules` directory in that modules, and so on. + +When all packages have been collected, it reads the package.json of each module and uses the `_location`-property +and the `_requiredBy`-property to recreate the complete dependency tree. + +* `_location` contains the location of the module in the directory tree. A module in `node_modules/packageA/node_modules/packageB` + has the location `/packageA/packageB` +* `_requiredBy` contains a list of module that are dependent on the current module. For each such module, it contains + the value of the `_location`-property. + +Once the packages is connected, the stats for each package are computed: + +* The number of dependencies is computed transitively across the tree. +* The total kilobytes (1024 bytes) is computed, include all dependencies. + The computation of file sizes assumes that only whole blocks are used, even by small files. The `blksize`-property + of the [fs.Stats-object]() is used as block size. If this value is missing (e.g. on Windows), a size of 4096 is + used. + +## Caveats + +In some cases, the dependencies in the `node_modules`-directory are tempered with. For example, [lerna](https://npmjs.com/package/lerna) +combines dependencies of multiple packages in the `node_modules`-directory of the root-project and removes +obsolete dependencies from the tree. This can lead to cycles in the dependency tree which are displayed in +the output like this: + +```txt +size: 42k... with-dependencies: 42k +└─┬ dep1@1.0.0, 42k, 3 deps + └─┬ dep1a@1.0.0, 42k, 3 deps + └─┬ dep2@1.0.0, 42k, 3 deps + └── dep1@1.0.0 (cycle detected) + +``` + + +Furthermore, this and the use of optional dependencies can lead to a situation where a package is `_requiredBy` +an existing dependency (i.e. a dependent package) but does not exist anymore in the tree. For those delete packages, +a dummy package is displayed in a separate tree. + +```txt +size: 42k... with-dependencies: 42k + +missing packages, that are referenced as dependent of an existing dependency +└─┬ /dep3, 42k, 1 deps + └── dep2@1.0.0, 42k, 0 deps + +``` + + +In this example, a module `node_modules/dep2` was found. The `_requireBy`-property shows that `node_modules/dep2` +is part of the tree, because it is a dependency of `node_modules/dep3`, which could not be found. + + + # License diff --git a/src/DependencyTree.js b/src/DependencyTree.js index 1c8a896..5273006 100644 --- a/src/DependencyTree.js +++ b/src/DependencyTree.js @@ -8,11 +8,12 @@ const {NullProgressHandler} = require('./progress') class DependencyTree { /** * @param {Package} prod package containing the production dependencies (and the files in the repository itself) - * @param {Package} dev package containing the dev-dependencies - * @param {Package} manual package containing the manually installed dependencies + * @param {Package[]} dev packages in the devDependencies + * @param {Package[]} manual manually installed packages (non-dependencies) + * @param {Package[]} missing packages whose dependent could not be found * @param {Package[]} all all packages in a node_modules directory, in a flat list. */ - constructor (rootPackage, prod, dev, manual, all) { + constructor (rootPackage, prod, dev, manual, missing, all) { this.rootPackage = rootPackage /** * @type {Package[]} @@ -26,6 +27,10 @@ class DependencyTree { * @type {Package[]} */ this.manual = manual + /** + * @type {Package[]} + */ + this.missing = missing /** * @type {Package[]} */ @@ -60,13 +65,14 @@ class DependencyTree { }) return deep({rootPackage, dependencies}).then(function ({rootPackage, dependencies}) { progressHandler.connectAll() - var {prod, dev, manual} = Package.connectAll(rootPackage, dependencies) + var {prod, dev, manual, missing} = Package.connectAll(rootPackage, dependencies) progressHandler.done() return new DependencyTree( rootPackage, - prod.dependencies, - dev.dependencies, - manual.dependencies, + prod, + dev, + manual, + missing, dependencies ) }) diff --git a/src/Package.js b/src/Package.js index f79fb5d..dbcabf3 100644 --- a/src/Package.js +++ b/src/Package.js @@ -33,15 +33,12 @@ class Package { * Create references to dependencies and dependents * contained in the packages-map. * - * @param {object} packages packages as returned by Package#indexByLocation + * @param {object} packageIndex packages as returned by Package#indexByLocation */ - connect (packages) { + connect (packageIndex) { if (this.packageJson._requiredBy) { this.packageJson._requiredBy.forEach((key) => { - const dependent = packages.get(key) - if (!dependent) { - throw new Error(`Could not find "${key}" in ${JSON.stringify(Array.from(packages.keys()))}`) - } + const dependent = packageIndex.get(key) || Package.dummyForMissingDependent(key, packageIndex) this.dependents.push(dependent) dependent.dependencies.push(this) }) @@ -96,26 +93,46 @@ class Package { */ static indexByLocation (packages) { const map = new Map(packages.map(p => [p.location(), p])) - map.set('#USER', new Package()) - map.set('#DEV:/', new Package()) + map.set('#USER', new Package({id: '#USER'})) + map.set('#DEV:/', new Package({id: '#DEV:/'})) + map.set('#MISSING', new Package({id: "#MISSING'"})) return map } /** - * - * @param {...(Package|Package[])} packages + * Create a dummy package for a missing dependent. + * @param location the location of the dummy package (i.e. the entry in the _requiredBy-tag of the dependency + * of this package) + * @param packageIndex the map of all packages (location -> package) + */ + static dummyForMissingDependent (location, packageIndex) { + const dummy = new Package({_id: location, _location: location}, new PackageStats(null, [])) + const sparePackage = packageIndex.get('#MISSING') + packageIndex.set(location, dummy) + dummy.dependents.push(sparePackage) + sparePackage.dependencies.push(dummy) + return dummy + } + + /** + * Connect a like of packages. The list may contain packages and arrays of packages. It will be flattened + * @param {(Package|Package[])[]} packages the list of packages or list of packages + * @return {object} an object containing dependencies of the following kind: + * prod ("dependencies"), dev ("devDependencies"), manual ("manually installed by the user"), + * missing ("implicitly found as dependent of another package") */ static connectAll (...packages) { debug('Connect all') // flatten const flatPackages = Array.prototype.concat.apply([], packages) - const index = Package.indexByLocation(flatPackages) - flatPackages.forEach((pkg) => pkg.connect(index)) + const packageIndex = Package.indexByLocation(flatPackages) + flatPackages.forEach((pkg) => pkg.connect(packageIndex)) debug('Done connect all') return { - prod: index.get('/'), - dev: index.get('#DEV:/'), - manual: index.get('#USER') + prod: packageIndex.get('/').dependencies, + dev: packageIndex.get('#DEV:/').dependencies, + manual: packageIndex.get('#USER').dependencies, + missing: packageIndex.get('#MISSING').dependencies } } } diff --git a/src/PackageStats.js b/src/PackageStats.js index 18fc81f..6859ecd 100644 --- a/src/PackageStats.js +++ b/src/PackageStats.js @@ -14,7 +14,7 @@ class PackageStats { /** * Create a new PackageStats object * @param {string} directory path to the package-json files - * @param {{file: string, stat: fs.Stats}} files files and stats in this package + * @param {{file: string, stat: fs.Stats}[]} files files and stats in this package */ constructor (directory, files) { /** @@ -22,7 +22,7 @@ class PackageStats { */ this.directory = directory /** - * @type {{file: string, stat: fs.Stats}} + * @type {{file: string, stat: fs.Stats}[]} */ this.files = files } diff --git a/src/index.js b/src/index.js index 7d19aa2..8a7eeb8 100755 --- a/src/index.js +++ b/src/index.js @@ -21,17 +21,26 @@ const {NullProgressHandler} = require('./progress') function analyze (cwd, options = {}) { return DependencyTree.loadFrom(path.join(cwd, 'package.json'), options.progress || new NullProgressHandler()) .then(function (tree) { - return archy({ + let result = archy({ label: `size: ${tree.rootPackage.stats.totalBlockSize() / 1024}k... with-dependencies: ${tree.rootPackage.totalStats().totalBlockSize() / 1024}k`, nodes: toArchy(tree.prod, options.depth, []) }) + if (tree.missing.length > 0) { + result += '\n' + archy({ + label: `missing packages, that are referenced as dependent of an existing dependency`, + nodes: toArchy(tree.missing, options.depth, []) + }) + } + return result }) } /** + * Create an archy-compatible object-structure of the dependency tree. * * @param pkgs * @param {number=} depth + * @param {string[]} cycleChecker list of "_location"s on the current path down the dependency tree. */ function toArchy (pkgs, depth, cycleChecker) { if (depth <= 0) return [] diff --git a/test/Package-spec.js b/test/Package-spec.js index 06b89c3..28bae5b 100644 --- a/test/Package-spec.js +++ b/test/Package-spec.js @@ -3,6 +3,13 @@ const chai = require('chai') chai.use(require('dirty-chai')) const expect = chai.expect + +function check (explanation) { + return { + expect: (value) => expect(value, explanation) + } +} + const {Package} = require('../src/Package') const {PackageStats} = require('../src/PackageStats') const deep = require('deep-aplus')(Promise) @@ -82,10 +89,35 @@ describe('The Package-class:', function () { expect(pkg2.dependents).to.deep.equal([pkg1]) }) - it('should throw an exception if a dependency could not be found', function () { - expect(function () { - dummy('one@1.0.0', '/one', ['/']).connect(new Map([['/c', new Package()]])) - }).to.throw(Error, 'Could not find "/" in ["/c"]') + it('should create dummy packages for missing dependents', function () { + const pkg1 = dummy('one@1.0.0', '/one', ['/three']) + const map = Package.indexByLocation([pkg1]) + pkg1.connect(map) + + // Checking dummy package + let dummyPackage = map.get('/three') + expect(dummyPackage.packageJson._id).to.equal('/three') + expect(dummyPackage.location()).to.equal('/three') + expect(dummyPackage.dependents).to.deep.equal([map.get('#MISSING')]) + expect(dummyPackage.dependencies).to.deep.equal([pkg1]) + }) + + it('should wire packages with one missing and one existing dependent to both dummy and existing package', function () { + const pkg1 = dummy('one@1.0.0', '/one', ['/three', '/']) + const root = dummy('root@1.0.0', '/', []) + const map = Package.indexByLocation([pkg1, root]) + pkg1.connect(map) + + // Checking dummy package + let dummyPackage = map.get('/three') + expect(dummyPackage.packageJson._id).to.equal('/three') + expect(dummyPackage.location()).to.equal('/three') + expect(dummyPackage.dependents).to.deep.equal([map.get('#MISSING')]) + expect(dummyPackage.dependencies).to.deep.equal([pkg1]) + + // Checking dependency wirings + expect(root.dependencies).to.deep.equal([pkg1]) + expect(pkg1.dependents).to.have.members([root, dummyPackage]) }) it('should ignore missing _requiredBy fields', function () { @@ -104,13 +136,39 @@ describe('The Package-class:', function () { const base = dummy('base@1.0.0', undefined, undefined) const pkg1 = dummy('one@1.0.0', '/one', ['/']) const pkg2 = dummy('two@1.0.0', '/two', ['/one']) - Package.connectAll(base, [pkg1, pkg2]) + const result = Package.connectAll(base, [pkg1, pkg2]) + expect(result).to.deep.equal({ + prod: [pkg1], + dev: [], + missing: [], + manual: [] + }) expect(base.dependencies).to.deep.equal([pkg1]) expect(pkg1.dependents).to.deep.equal([base]) expect(pkg1.dependencies).to.deep.equal([pkg2]) expect(pkg2.dependents).to.deep.equal([pkg1]) }) + + it('should connect missing dependents to "missing"', function () { + const base = dummy('base@1.0.0', undefined, undefined) + const pkg1 = dummy('one@1.0.0', '/one', ['/three', '/']) + const pkg2 = dummy('two@1.0.0', '/two', ['/three']) + // /three is missing + + const result = Package.connectAll(base, [pkg1, pkg2]) + + check('the base package').expect(result.prod).to.deep.equal(base.dependencies) + check('number of missing packages').expect(result.missing.length).to.equal(1) + const dummyPkg3 = result.missing[0] + check('id of missing package').expect(dummyPkg3.packageJson._id).to.equal('/three') + check('location of missing package').expect(dummyPkg3.location()).to.equal('/three') + check('dependencies of missing package /three').expect(dummyPkg3.dependencies).to.have.members([pkg1, pkg2]) + + check('dependencies of the base package').expect(base.dependencies).to.deep.equal([pkg1]) + check('dependents of pkg1').expect(pkg1.dependents).to.have.members([base, dummyPkg3]) + check('dependents of pkg2').expect(pkg2.dependents).to.deep.equal([dummyPkg3]) + }) }) describe('The #totalDependencies method', function () { diff --git a/test/fixtures/moduleWithCyclicDeps/node_modules/dep2/package.json b/test/fixtures/moduleWithCyclicDeps/node_modules/dep2/package.json new file mode 100644 index 0000000..a2c3ae3 --- /dev/null +++ b/test/fixtures/moduleWithCyclicDeps/node_modules/dep2/package.json @@ -0,0 +1,6 @@ +{ + "name": "dep2", + "_id": "dep2@1.0.0", + "_location": "/dep2", + "_requiredBy": ["/dep1a"] +} \ No newline at end of file diff --git a/test/fixtures/moduleWithCyclicDeps/node_modules/devdep1/package.json b/test/fixtures/moduleWithCyclicDeps/node_modules/devdep1/package.json new file mode 100644 index 0000000..81e73c6 --- /dev/null +++ b/test/fixtures/moduleWithCyclicDeps/node_modules/devdep1/package.json @@ -0,0 +1,6 @@ +{ + "name": "devdep1", + "_id": "devdep1@1.0.0", + "_location": "/devdep1", + "_requiredBy": ["#DEV:/"] +} \ No newline at end of file diff --git a/test/fixtures/moduleWithMissingDependent.txt b/test/fixtures/moduleWithMissingDependent.txt new file mode 100644 index 0000000..0299715 --- /dev/null +++ b/test/fixtures/moduleWithMissingDependent.txt @@ -0,0 +1,7 @@ +size: 42k... with-dependencies: 42k +└── dep1@1.0.0, 42k, 0 deps + +missing packages, that are referenced as dependent of an existing dependency +└─┬ /dep3, 42k, 2 deps + └─┬ dep2@1.0.0, 42k, 1 deps + └── dep1@1.0.0, 42k, 0 deps diff --git a/test/fixtures/moduleWithMissingDependent/index.js b/test/fixtures/moduleWithMissingDependent/index.js new file mode 100644 index 0000000..e69de29 diff --git a/test/fixtures/moduleWithMissingDependent/node_modules/dep1/package.json b/test/fixtures/moduleWithMissingDependent/node_modules/dep1/package.json new file mode 100644 index 0000000..adac345 --- /dev/null +++ b/test/fixtures/moduleWithMissingDependent/node_modules/dep1/package.json @@ -0,0 +1,6 @@ +{ + "name": "dep1", + "_id": "dep1@1.0.0", + "_location": "/dep1", + "_requiredBy": ["/","/dep2"] +} diff --git a/test/fixtures/moduleWithMissingDependent/node_modules/dep2/package.json b/test/fixtures/moduleWithMissingDependent/node_modules/dep2/package.json new file mode 100644 index 0000000..92193f5 --- /dev/null +++ b/test/fixtures/moduleWithMissingDependent/node_modules/dep2/package.json @@ -0,0 +1,6 @@ +{ + "name": "dep2", + "_id": "dep2@1.0.0", + "_location": "/dep2", + "_requiredBy": ["/dep3"] +} \ No newline at end of file diff --git a/test/fixtures/moduleWithMissingDependent/package.json b/test/fixtures/moduleWithMissingDependent/package.json new file mode 100644 index 0000000..2e3d320 --- /dev/null +++ b/test/fixtures/moduleWithMissingDependent/package.json @@ -0,0 +1,6 @@ +{ + "name": "moduleWithDeps", + "version": "1.0.0", + "dependencies": { + } +} diff --git a/test/index-spec.js b/test/index-spec.js index a9af6ad..0489500 100644 --- a/test/index-spec.js +++ b/test/index-spec.js @@ -37,6 +37,15 @@ describe('The index-function (module main function):', function () { expect(k24(result)).to.equal(k24(fs.readFileSync(`test/fixtures/moduleWithCyclicDeps.txt`, 'utf-8'))) }) }) + + it('should handle missing dependents gracefully', function () { + return analyze('test/fixtures/moduleWithMissingDependent') + .then((result) => { + // The size numbers in this test are irrelevant. In order to keep the test compatible on multiple platforms, + // all sizes are set to 42k + expect(k24(result)).to.equal(k24(fs.readFileSync(`test/fixtures/moduleWithMissingDependent.txt`, 'utf-8'))) + }) + }) }) /**