From 9d26bb748c0e17a7bbf24704b168e9eae99ddead Mon Sep 17 00:00:00 2001 From: Patrik Henningsson Date: Tue, 6 Sep 2016 08:42:28 +0200 Subject: [PATCH 1/4] Bump dependency --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 3438d54f..189d59cf 100644 --- a/package.json +++ b/package.json @@ -42,7 +42,7 @@ "commander": "^2.9.0", "commondir": "^1.0.1", "debug": "^2.2.0", - "dependency-tree": "^5.5.1", + "dependency-tree": "^5.5.3", "graphviz": "^0.0.8", "mz": "^2.4.0", "rc": "^1.1.6", From 8c56037570f53ed0c0560fcff391efe05227dc1b Mon Sep 17 00:00:00 2001 From: Patrik Henningsson Date: Tue, 6 Sep 2016 08:58:52 +0200 Subject: [PATCH 2/4] Rename commonjs to cjs --- package.json | 2 +- test/api.js | 26 +++++++++---------- test/{commonjs.js => cjs.js} | 2 +- test/{commonjs => cjs}/a.js | 0 test/{commonjs => cjs}/b.js | 0 test/{commonjs => cjs}/both.js | 0 test/{commonjs => cjs}/c.js | 0 test/{commonjs => cjs}/chained.js | 0 test/{commonjs => cjs}/circular/a.js | 0 test/{commonjs => cjs}/circular/b.js | 0 test/{commonjs => cjs}/circular/c.js | 0 test/{commonjs => cjs}/circular/d.js | 0 test/{commonjs => cjs}/core.js | 0 test/{commonjs => cjs}/error.js | 0 test/{commonjs => cjs}/multibase/1/a.js | 0 test/{commonjs => cjs}/multibase/2/b.js | 0 test/{commonjs => cjs}/nested.js | 0 test/{commonjs => cjs}/node_modules/a.js | 0 test/{commonjs => cjs}/node_modules/b.js | 0 test/{commonjs => cjs}/node_modules/c.js | 0 test/{commonjs => cjs}/node_modules/doom.js | 0 .../{commonjs => cjs}/node_modules/events2.js | 0 test/{commonjs => cjs}/node_modules/y.js | 0 test/{commonjs => cjs}/normal/a.js | 0 test/{commonjs => cjs}/normal/d.js | 0 test/{commonjs => cjs}/normal/sub/b.js | 0 test/{commonjs => cjs}/normal/sub/c.js | 0 test/{commonjs => cjs}/npm.js | 0 test/{commonjs => cjs}/strings.js | 0 test/{commonjs => cjs}/word.js | 0 test/flow.js | 2 +- test/flow/{commonjs => cjs}/calc.js | 0 test/flow/{commonjs => cjs}/math.js | 0 33 files changed, 16 insertions(+), 16 deletions(-) rename test/{commonjs.js => cjs.js} (97%) rename test/{commonjs => cjs}/a.js (100%) rename test/{commonjs => cjs}/b.js (100%) rename test/{commonjs => cjs}/both.js (100%) rename test/{commonjs => cjs}/c.js (100%) rename test/{commonjs => cjs}/chained.js (100%) rename test/{commonjs => cjs}/circular/a.js (100%) rename test/{commonjs => cjs}/circular/b.js (100%) rename test/{commonjs => cjs}/circular/c.js (100%) rename test/{commonjs => cjs}/circular/d.js (100%) rename test/{commonjs => cjs}/core.js (100%) rename test/{commonjs => cjs}/error.js (100%) rename test/{commonjs => cjs}/multibase/1/a.js (100%) rename test/{commonjs => cjs}/multibase/2/b.js (100%) rename test/{commonjs => cjs}/nested.js (100%) rename test/{commonjs => cjs}/node_modules/a.js (100%) rename test/{commonjs => cjs}/node_modules/b.js (100%) rename test/{commonjs => cjs}/node_modules/c.js (100%) rename test/{commonjs => cjs}/node_modules/doom.js (100%) rename test/{commonjs => cjs}/node_modules/events2.js (100%) rename test/{commonjs => cjs}/node_modules/y.js (100%) rename test/{commonjs => cjs}/normal/a.js (100%) rename test/{commonjs => cjs}/normal/d.js (100%) rename test/{commonjs => cjs}/normal/sub/b.js (100%) rename test/{commonjs => cjs}/normal/sub/c.js (100%) rename test/{commonjs => cjs}/npm.js (100%) rename test/{commonjs => cjs}/strings.js (100%) rename test/{commonjs => cjs}/word.js (100%) rename test/flow/{commonjs => cjs}/calc.js (100%) rename test/flow/{commonjs => cjs}/math.js (100%) diff --git a/package.json b/package.json index 189d59cf..12a6ed16 100644 --- a/package.json +++ b/package.json @@ -34,7 +34,7 @@ "lint": "eslint bin/cli.js lib test/*.js", "debug": "node bin/cli.js --debug bin lib", "generate": "npm run generate:small && npm run generate:madge", - "generate:small": "bin/cli.js --image /tmp/simple.svg test/commonjs/circular/a.js", + "generate:small": "bin/cli.js --image /tmp/simple.svg test/cjs/circular/a.js", "generate:madge": "bin/cli.js --image /tmp/madge.svg bin lib" }, "dependencies": { diff --git a/test/api.js b/test/api.js index c24f4868..2172e4e2 100644 --- a/test/api.js +++ b/test/api.js @@ -16,7 +16,7 @@ describe('API', () => { }); it('returns a Promise', () => { - madge(__dirname + '/commonjs/a.js').should.be.Promise(); // eslint-disable-line new-cap + madge(__dirname + '/cjs/a.js').should.be.Promise(); // eslint-disable-line new-cap }); it('throws error if file or directory does not exists', (done) => { @@ -27,7 +27,7 @@ describe('API', () => { }); it('takes single file as path', (done) => { - madge(__dirname + '/commonjs/a.js').then((res) => { + madge(__dirname + '/cjs/a.js').then((res) => { res.obj().should.eql({ 'a': ['b', 'c'], 'b': ['c'], @@ -38,7 +38,7 @@ describe('API', () => { }); it('takes an array of files as path and combines the result', (done) => { - madge([__dirname + '/commonjs/a.js', __dirname + '/commonjs/normal/d.js']).then((res) => { + madge([__dirname + '/cjs/a.js', __dirname + '/cjs/normal/d.js']).then((res) => { res.obj().should.eql({ 'a': ['b', 'c'], 'b': ['c'], @@ -50,7 +50,7 @@ describe('API', () => { }); it('take a single directory as path and find files in it', (done) => { - madge(__dirname + '/commonjs/normal').then((res) => { + madge(__dirname + '/cjs/normal').then((res) => { res.obj().should.eql({ 'a': ['sub/b'], 'd': [], @@ -62,7 +62,7 @@ describe('API', () => { }); it('takes an array of directories as path and compute the basedir correctly', (done) => { - madge([__dirname + '/commonjs/multibase/1', __dirname + '/commonjs/multibase/2']).then((res) => { + madge([__dirname + '/cjs/multibase/1', __dirname + '/cjs/multibase/2']).then((res) => { res.obj().should.eql({ '1/a': [], '2/b': [] @@ -89,7 +89,7 @@ describe('API', () => { }); it('can exclude modules using RegExp', (done) => { - madge(__dirname + '/commonjs/a.js', { + madge(__dirname + '/cjs/a.js', { excludeRegExp: ['^b$'] }).then((res) => { res.obj().should.eql({ @@ -102,7 +102,7 @@ describe('API', () => { describe('obj()', () => { it('returns dependency object', (done) => { - madge(__dirname + '/commonjs/a.js').then((res) => { + madge(__dirname + '/cjs/a.js').then((res) => { res.obj().should.eql({ a: ['b', 'c'], b: ['c'], @@ -115,7 +115,7 @@ describe('API', () => { describe('dot()', () => { it('returns a promise resolved with graphviz DOT output', (done) => { - madge(__dirname + '/commonjs/b.js') + madge(__dirname + '/cjs/b.js') .then((res) => res.dot()) .then((output) => { output.should.eql('digraph G {\n "b";\n "c";\n "b" -> "c";\n}\n'); @@ -127,7 +127,7 @@ describe('API', () => { describe('depends()', () => { it('returns modules that depends on another', (done) => { - madge(__dirname + '/commonjs/a.js').then((res) => { + madge(__dirname + '/cjs/a.js').then((res) => { res.depends('c').should.eql(['a', 'b']); done(); }).catch(done); @@ -146,7 +146,7 @@ describe('API', () => { }); it('rejects if a filename is not supplied', (done) => { - madge(__dirname + '/commonjs/a.js') + madge(__dirname + '/cjs/a.js') .then((res) => res.image()) .catch((err) => { err.message.should.eql('imagePath not provided'); @@ -155,7 +155,7 @@ describe('API', () => { }); it('rejects on unsupported image format', (done) => { - madge(__dirname + '/commonjs/a.js') + madge(__dirname + '/cjs/a.js') .then((res) => res.image('image.zyx')) .catch((err) => { err.message.should.match(/Format: "zyx" not recognized/); @@ -164,7 +164,7 @@ describe('API', () => { }); it('rejects if graphviz is not installed', (done) => { - madge(__dirname + '/commonjs/a.js', {graphVizPath: '/invalid/path'}) + madge(__dirname + '/cjs/a.js', {graphVizPath: '/invalid/path'}) .then((res) => res.image('image.png')) .catch((err) => { err.message.should.match(/Could not execute .*gvpr \-V/); @@ -173,7 +173,7 @@ describe('API', () => { }); it('writes image to file', (done) => { - madge(__dirname + '/commonjs/a.js') + madge(__dirname + '/cjs/a.js') .then((res) => res.image(imagePath)) .then((writtenImagePath) => { writtenImagePath.should.eql(imagePath); diff --git a/test/commonjs.js b/test/cjs.js similarity index 97% rename from test/commonjs.js rename to test/cjs.js index 570dcb0c..4abe5a59 100644 --- a/test/commonjs.js +++ b/test/cjs.js @@ -5,7 +5,7 @@ const madge = require('../lib/api'); require('should'); describe('CommonJS', () => { - const dir = __dirname + '/commonjs'; + const dir = __dirname + '/cjs'; it('finds recursive dependencies', (done) => { madge(dir + '/normal/a.js').then((res) => { diff --git a/test/commonjs/a.js b/test/cjs/a.js similarity index 100% rename from test/commonjs/a.js rename to test/cjs/a.js diff --git a/test/commonjs/b.js b/test/cjs/b.js similarity index 100% rename from test/commonjs/b.js rename to test/cjs/b.js diff --git a/test/commonjs/both.js b/test/cjs/both.js similarity index 100% rename from test/commonjs/both.js rename to test/cjs/both.js diff --git a/test/commonjs/c.js b/test/cjs/c.js similarity index 100% rename from test/commonjs/c.js rename to test/cjs/c.js diff --git a/test/commonjs/chained.js b/test/cjs/chained.js similarity index 100% rename from test/commonjs/chained.js rename to test/cjs/chained.js diff --git a/test/commonjs/circular/a.js b/test/cjs/circular/a.js similarity index 100% rename from test/commonjs/circular/a.js rename to test/cjs/circular/a.js diff --git a/test/commonjs/circular/b.js b/test/cjs/circular/b.js similarity index 100% rename from test/commonjs/circular/b.js rename to test/cjs/circular/b.js diff --git a/test/commonjs/circular/c.js b/test/cjs/circular/c.js similarity index 100% rename from test/commonjs/circular/c.js rename to test/cjs/circular/c.js diff --git a/test/commonjs/circular/d.js b/test/cjs/circular/d.js similarity index 100% rename from test/commonjs/circular/d.js rename to test/cjs/circular/d.js diff --git a/test/commonjs/core.js b/test/cjs/core.js similarity index 100% rename from test/commonjs/core.js rename to test/cjs/core.js diff --git a/test/commonjs/error.js b/test/cjs/error.js similarity index 100% rename from test/commonjs/error.js rename to test/cjs/error.js diff --git a/test/commonjs/multibase/1/a.js b/test/cjs/multibase/1/a.js similarity index 100% rename from test/commonjs/multibase/1/a.js rename to test/cjs/multibase/1/a.js diff --git a/test/commonjs/multibase/2/b.js b/test/cjs/multibase/2/b.js similarity index 100% rename from test/commonjs/multibase/2/b.js rename to test/cjs/multibase/2/b.js diff --git a/test/commonjs/nested.js b/test/cjs/nested.js similarity index 100% rename from test/commonjs/nested.js rename to test/cjs/nested.js diff --git a/test/commonjs/node_modules/a.js b/test/cjs/node_modules/a.js similarity index 100% rename from test/commonjs/node_modules/a.js rename to test/cjs/node_modules/a.js diff --git a/test/commonjs/node_modules/b.js b/test/cjs/node_modules/b.js similarity index 100% rename from test/commonjs/node_modules/b.js rename to test/cjs/node_modules/b.js diff --git a/test/commonjs/node_modules/c.js b/test/cjs/node_modules/c.js similarity index 100% rename from test/commonjs/node_modules/c.js rename to test/cjs/node_modules/c.js diff --git a/test/commonjs/node_modules/doom.js b/test/cjs/node_modules/doom.js similarity index 100% rename from test/commonjs/node_modules/doom.js rename to test/cjs/node_modules/doom.js diff --git a/test/commonjs/node_modules/events2.js b/test/cjs/node_modules/events2.js similarity index 100% rename from test/commonjs/node_modules/events2.js rename to test/cjs/node_modules/events2.js diff --git a/test/commonjs/node_modules/y.js b/test/cjs/node_modules/y.js similarity index 100% rename from test/commonjs/node_modules/y.js rename to test/cjs/node_modules/y.js diff --git a/test/commonjs/normal/a.js b/test/cjs/normal/a.js similarity index 100% rename from test/commonjs/normal/a.js rename to test/cjs/normal/a.js diff --git a/test/commonjs/normal/d.js b/test/cjs/normal/d.js similarity index 100% rename from test/commonjs/normal/d.js rename to test/cjs/normal/d.js diff --git a/test/commonjs/normal/sub/b.js b/test/cjs/normal/sub/b.js similarity index 100% rename from test/commonjs/normal/sub/b.js rename to test/cjs/normal/sub/b.js diff --git a/test/commonjs/normal/sub/c.js b/test/cjs/normal/sub/c.js similarity index 100% rename from test/commonjs/normal/sub/c.js rename to test/cjs/normal/sub/c.js diff --git a/test/commonjs/npm.js b/test/cjs/npm.js similarity index 100% rename from test/commonjs/npm.js rename to test/cjs/npm.js diff --git a/test/commonjs/strings.js b/test/cjs/strings.js similarity index 100% rename from test/commonjs/strings.js rename to test/cjs/strings.js diff --git a/test/commonjs/word.js b/test/cjs/word.js similarity index 100% rename from test/commonjs/word.js rename to test/cjs/word.js diff --git a/test/flow.js b/test/flow.js index df0cef40..9e5664c3 100644 --- a/test/flow.js +++ b/test/flow.js @@ -18,7 +18,7 @@ describe('Flow', () => { }); it('extracts CommonsJS module dependencies', (done) => { - madge(dir + '/commonjs/calc.js').then((res) => { + madge(dir + '/cjs/calc.js').then((res) => { res.obj().should.eql({ 'math': [], 'calc': ['math'] diff --git a/test/flow/commonjs/calc.js b/test/flow/cjs/calc.js similarity index 100% rename from test/flow/commonjs/calc.js rename to test/flow/cjs/calc.js diff --git a/test/flow/commonjs/math.js b/test/flow/cjs/math.js similarity index 100% rename from test/flow/commonjs/math.js rename to test/flow/cjs/math.js From fc67d0060d3050357f161cf5a86973a53b82910c Mon Sep 17 00:00:00 2001 From: Patrik Henningsson Date: Tue, 6 Sep 2016 09:39:11 +0200 Subject: [PATCH 3/4] Cache paths when converting tree for better performance --- lib/tree.js | 155 ++++++++++++++++++++++++++++------------------------ 1 file changed, 85 insertions(+), 70 deletions(-) diff --git a/lib/tree.js b/lib/tree.js index 2d39cea9..15d3a1db 100644 --- a/lib/tree.js +++ b/lib/tree.js @@ -1,5 +1,6 @@ 'use strict'; +const os = require('os'); const fs = require('mz/fs'); const path = require('path'); const commondir = require('commondir'); @@ -7,6 +8,12 @@ const walk = require('walkdir'); const dependencyTree = require('dependency-tree'); const log = require('./log'); +/** + * Check if running on Windows. + * @type {Boolean} + */ +const isWin = (os.platform() === 'win32'); + class Tree { /** * Class constructor. @@ -30,47 +37,17 @@ class Tree { } /** - * Generate the tree from the given files - * @param {Array} files - * @return {Object} + * Set the base directory (compute the common one if multiple). + * @param {Array} dirs */ - generateTree(files) { - const depTree = {}; - const visited = {}; - - files.forEach((file) => { - if (visited[file]) { - return; - } - - Object.assign(depTree, dependencyTree({ - filename: file, - directory: this.baseDir, - requireConfig: this.config.requireConfig, - webpackConfig: this.config.webpackConfig, - visited: visited, - filter: this.filterPath.bind(this) - })); - }); - - let tree = this.flatten(depTree); - - if (this.config.excludeRegExp) { - tree = this.exclude(tree, this.config.excludeRegExp); + setBaseDir(dirs) { + if (this.config.baseDir) { + this.baseDir = path.resolve(this.config.baseDir); + } else { + this.baseDir = commondir(dirs); } - tree = this.sort(tree); - - return tree; - } - - /** - * Filter out some paths from found files - * @param {String} path - * @return {Boolean} - */ - filterPath(path) { - return this.config.includeNpm || path.indexOf('node_modules') < 0; + log('using base directory %s', this.baseDir); } /** @@ -120,61 +97,99 @@ class Tree { } /** - * Set the base directory (compute the common one if multiple). - * @param {Array} dirs + * Generate the tree from the given files + * @param {Array} files + * @return {Object} */ - setBaseDir(dirs) { - if (this.config.baseDir) { - this.baseDir = path.resolve(this.config.baseDir); - } else { - this.baseDir = commondir(dirs); + generateTree(files) { + const depTree = {}; + const visited = {}; + + files.forEach((file) => { + if (visited[file]) { + return; + } + + Object.assign(depTree, dependencyTree({ + filename: file, + directory: this.baseDir, + requireConfig: this.config.requireConfig, + webpackConfig: this.config.webpackConfig, + visited: visited, + filter: this.filterPath.bind(this) + })); + }); + + let tree = this.convertTree(depTree, {}, {}); + + if (this.config.excludeRegExp) { + tree = this.exclude(tree, this.config.excludeRegExp); } - log('using base directory %s', this.baseDir); - } + tree = this.sort(tree); + return tree; + } /** - * Flatten deep tree produced by `dependency-tree`. - * @param {Object} deepTree - * @param {Object} [tree] + * Convert deep tree produced by dependency-tree to a + * shallow (one level deep) tree used by madge. + * @param {Object} depTree + * @param {Object} tree + * @param {Object} pathCache * @return {Object} */ - flatten(deepTree, tree) { - tree = tree || {}; - - Object - .keys(deepTree) - .forEach((key) => { - const id = this.processPath(key); - - if (!tree[id]) { - tree[id] = Object - .keys(deepTree[key]) - .map((dep) => this.processPath(dep)); + convertTree(depTree, tree, pathCache) { + for (const key in depTree) { + const id = this.processPath(key, pathCache); + + if (!tree[id]) { + tree[id] = []; + + for (const dep in depTree[key]) { + tree[id].push(this.processPath(dep, pathCache)); } + } - this.flatten(deepTree[key], tree); - }); + this.convertTree(depTree[key], tree, pathCache); + } return tree; } /** - * Process path. + * Process absolute path and return a shorter one. * @param {String} absPath + * @param {Object} cache * @return {String} */ - processPath(absPath) { - absPath = path.relative(this.baseDir, absPath); + processPath(absPath, cache) { + if (cache[absPath]) { + return cache[absPath]; + } + + let relPath = path.relative(this.baseDir, absPath); if (!this.config.showFileExtension) { - absPath = absPath.replace(/\.\w+$/, ''); + relPath = relPath.substr(0, relPath.lastIndexOf('.')); + } + + if (isWin) { + relPath = relPath.replace(/\\/g, '/'); } - absPath = absPath.replace(/\\/g, '/'); + cache[absPath] = relPath; - return absPath; + return relPath; + } + + /** + * Filter out some paths from found files + * @param {String} path + * @return {Boolean} + */ + filterPath(path) { + return this.config.includeNpm || path.indexOf('node_modules') < 0; } /** From 08f9150561373a915daec6d4c103c1d8cd616525 Mon Sep 17 00:00:00 2001 From: Patrik Henningsson Date: Tue, 6 Sep 2016 12:50:26 +0200 Subject: [PATCH 4/4] Fix failing JSX test --- test/jsx/basic.jsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/jsx/basic.jsx b/test/jsx/basic.jsx index 3b037beb..f8c55934 100644 --- a/test/jsx/basic.jsx +++ b/test/jsx/basic.jsx @@ -1,5 +1,5 @@ import React from 'react'; -import other from './other'; +import other from './other.jsx'; export default React.createClass({ render: function() {