Skip to content

Commit

Permalink
Check for cycles when traversing the dependency tree
Browse files Browse the repository at this point in the history
- Cycles may occur in some cases (dependencies like
  a -> b -> c -> a)
- This commit makes #totalStats() and #totalDependencies() safe
  for cyclic.
- The method Package#collectDependencies can be used to avoid cycles
  in future aggregation methods.
  • Loading branch information
nknapp committed May 7, 2017
1 parent 3aabcfe commit cc9677b
Show file tree
Hide file tree
Showing 2 changed files with 32 additions and 5 deletions.
20 changes: 15 additions & 5 deletions src/Package.js
Expand Up @@ -12,7 +12,7 @@ const debug = require('debug')('analyze-module-size:Package')
*/
class Package {
/**
* Create a new Packageobject
* Create a new Package object
* @param {object} packageJson the parsed package.json file
* @param {PackageStats} stats the statistics object
*/
Expand Down Expand Up @@ -49,13 +49,23 @@ class Package {
}

totalStats () {
return this.stats.merge(this.dependencies.map((dep) => {
return dep.totalStats()
}))
return this.stats.merge(Array.from(this.collectDependencies()).map((dep) => dep.stats))
}

totalDependencies () {
return this.dependencies.reduce((sum, dep) => sum + dep.totalDependencies() + 1, 0)
return this.collectDependencies().size
}

/**
* Reduce-function that not reduces the direct and recursive dependencies
*/
collectDependencies (collector = new Set()) {
this.dependencies.forEach(function (dep) {
if (collector.has(dep)) return
collector.add(dep)
dep.collectDependencies(collector)
})
return collector
}

location () {
Expand Down
17 changes: 17 additions & 0 deletions test/Package-spec.js
Expand Up @@ -121,6 +121,15 @@ describe('The Package-class:', function () {
Package.connectAll(base, [pkg1, pkg2, pkg3])
expect(base.totalDependencies()).to.equal(2)
})

it('should ignore cyclic dependencies', 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', ['/one'])
const pkg3 = dummy('dev@1.0.0', '/three', ['/two'])
Package.connectAll(base, [pkg1, pkg2, pkg3])
expect(base.totalDependencies()).to.equal(3)
})
})

describe('The #totalStats method', function () {
Expand All @@ -145,6 +154,14 @@ describe('The Package-class:', function () {
Package.connectAll(base, [pkg1, pkg2, pkg3])
expect(base.totalStats()).to.deep.equal(new PackageStats('/base@1.0.0', []).merge([stats1, stats2]))
})

it('should stop traversing at dependency cycles', function () {
const base = dummy('base@1.0.0', undefined, undefined)
const pkg1 = dummy('one@1.0.0', '/one', ['/', '/two'], stats1)
const pkg2 = dummy('two@1.0.0', '/two', ['/one'], stats2)
Package.connectAll(base, [pkg1, pkg2])
expect(base.totalStats()).to.deep.equal(new PackageStats('/base@1.0.0', []).merge([stats1, stats2]))
})
})
})

Expand Down

0 comments on commit cc9677b

Please sign in to comment.