Skip to content

Commit

Permalink
IMPROVED: correctly detect circular dependencies when using path alia…
Browse files Browse the repository at this point in the history
…ses (amd)

ADDED: new test for circular dependencies with path aliases
  • Loading branch information
warpdesign committed Sep 4, 2014
1 parent dc35cf4 commit 324b12b
Show file tree
Hide file tree
Showing 8 changed files with 64 additions and 9 deletions.
4 changes: 2 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ Only required if you want to generate the visual graphs using [Graphviz](http://
- {Boolean} **breakOnError**. True if the parser should stop on parse errors and when modules are missing, false otherwise. Defaults to false.
- {Boolean} **optimized**. True if the parser should read modules from a optimized file (r.js). Defaults to false.
- {String} **mainRequireModule**. Name of the module if parsing an optimized file (r.js), where the main file used `require()` instead of `define`. Defaults to `''`.
- {String} **requireConfig**. Path to RequireJS config used to find shim dependencies. Not used by default.
- {String} **requireConfig**. Path to RequireJS config used to find shim dependencies and path aliases. Not used by default.
- {Function} **onParseFile**. Function to be called when parsing a file (argument will be an object with "filename" and "src" property set).
- {Function} **onAddModule** . Function to be called when adding a module to the module tree (argument will be an object with "id" and "dependencies" property set).

Expand Down Expand Up @@ -133,7 +133,7 @@ Get an image representation of the module dependency graph.
-n, --no-colors skip colors in output and images
-r, --read skip scanning folders and read JSON from stdin
-C, --config <filename> provide a config file
-R, --require-config <filename> include shim dependencies found in RequireJS config file
-R, --require-config <filename> include shim dependencies and path aliases found in RequireJS config file
-O, --optimized if given file is optimized with r.js
-M --main-require-module name of the primary RequireJS module, if it's included with `require()`
-j --json output dependency tree in json
Expand Down
4 changes: 2 additions & 2 deletions bin/madge
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ program
.option('-n, --no-colors', 'skip colors in output and images', false)
.option('-r, --read', 'skip scanning folders and read JSON from stdin')
.option('-C, --config <filename>', 'provide a config file')
.option('-R, --require-config <filename>', 'include shim dependencies found in RequireJS config file')
.option('-R, --require-config <filename>', 'include shim dependencies and paths found in RequireJS config file')
.option('-O, --optimized', 'if given file is optimized with r.js', false)
.option('-M, --main-require-module', 'name of the primary RequireJS module, if it\'s included with `require()`', '')
.option('-j --json', 'output dependency tree in json')
Expand Down Expand Up @@ -128,4 +128,4 @@ function run() {
output: program.output
});
}
}
}
17 changes: 16 additions & 1 deletion lib/madge.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,20 @@ function mergeTrees(a, b) {
});
});
}
/**
* Replace alias found inside tree with alias
* @param {Object} tree
* @param {Object} alias list
*/
function convertAliases(tree, aliases) {
Object.keys(tree).forEach(function (id) {
tree[id].forEach(function (moduleName, i, array) {
if (aliases[moduleName]) {
array[i] = aliases[moduleName];
}
});
});
}

/**
* Expose factory function.
Expand Down Expand Up @@ -74,6 +88,7 @@ function Madge(src, opts) {

if (this.opts.requireConfig) {
mergeTrees(tree, requirejs.getShimDepsFromConfig(this.opts.requireConfig, this.opts.exclude));
convertAliases(tree, requirejs.getPathsFromConfig(this.opts.requireConfig, this.opts.exclude));
}

this.tree = tree;
Expand Down Expand Up @@ -133,4 +148,4 @@ Madge.prototype.dot = function () {
*/
Madge.prototype.image = function (opts, callback) {
graph.image(this.tree, opts, callback);
};
};
24 changes: 23 additions & 1 deletion lib/requirejs.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,4 +30,26 @@ module.exports.getShimDepsFromConfig = function (filename, exclude) {
}

return deps;
};
};

/**
* Read path definitions from RequireJS config.
* @param {String} filename
* @param {String} [exclude]
*/
module.exports.getPathsFromConfig = function (filename, exclude) {
var paths = {},
config = parse.findConfig(filename, fs.readFileSync(filename, 'utf8')),
excludeRegex = exclude ? new RegExp(exclude) : false,
isIncluded = function (key) {
return !(excludeRegex && key.match(excludeRegex));
};

if (config.paths) {
Object.keys(config.paths).filter(isIncluded).forEach(function (key) {
paths[key] = config.paths[key];
});
}

return paths;
};
13 changes: 10 additions & 3 deletions test/amd.js
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ describe('module format (AMD)', function () {
madge([__dirname + '/files/amd/requirejs/a.js'], {
format: 'amd',
requireConfig: __dirname + '/files/amd/requirejs/config.js'
}).obj().should.eql({ a: [ 'jquery' ], 'jquery': [], 'jquery.foo': [ 'jquery' ], 'jquery.bar': [ 'jquery' ], 'baz': [ 'quux' ], 'quux': [] });
}).obj().should.eql({ a: [ 'vendor/jquery-2.0.3' ], 'jquery': [], 'jquery.foo': [ 'vendor/jquery-2.0.3' ], 'jquery.bar': [ 'vendor/jquery-2.0.3' ], 'baz': [ 'vendor/quux' ], 'quux': [] });
});

it('should be able to exclude modules', function () {
Expand All @@ -53,7 +53,7 @@ describe('module format (AMD)', function () {
format: 'amd',
requireConfig: __dirname + '/files/amd/requirejs/config.js',
exclude: '^jquery.foo|quux$'
}).obj().should.eql({ a: [ 'jquery' ], 'jquery': [], 'jquery.bar': [ 'jquery' ] , 'baz': []});
}).obj().should.eql({ a: [ 'vendor/jquery-2.0.3' ], 'jquery': [], 'jquery.bar': [ 'vendor/jquery-2.0.3' ] , 'baz': []});
});

it('should tackle errors in files', function () {
Expand Down Expand Up @@ -86,6 +86,13 @@ describe('module format (AMD)', function () {
}).circular().getArray().should.eql([ [ 'a', 'foo/b' ] ]);
});

it('should find circular dependencies with alias', function () {
madge([__dirname + '/files/amd/circularAlias'], {
format: 'amd',
requireConfig: __dirname + '/files/amd/circularAlias/config.js'
}).circular().getArray().should.eql([ [ 'dos', 'x86' ] ]);
});

it('should find modules that depends on another', function () {
madge([__dirname + '/files/amd/ok'], {
format: 'amd'
Expand All @@ -110,4 +117,4 @@ describe('module format (AMD)', function () {
breakOnError: true
}).obj().should.eql({ plugin: [ 'ok/a' ] });
});
});
});
7 changes: 7 additions & 0 deletions test/files/amd/circularAlias/config.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
require.config({
paths: {
'cpu': 'x86',
'jsdos':'dos'
}
});

2 changes: 2 additions & 0 deletions test/files/amd/circularAlias/dos.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
define(['cpu'], function(cpu) {
});
2 changes: 2 additions & 0 deletions test/files/amd/circularAlias/x86.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
define(['jsdos'], function(dos) {
});

0 comments on commit 324b12b

Please sign in to comment.