From c802fea8c51eb999f9d5fd81e37ac01f0fcb84b1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Felix=20Bo=CC=88hm?= Date: Fri, 7 Sep 2012 18:44:11 +0200 Subject: [PATCH 01/20] use console.log instead of util.puts also fixed some linting errors (semicolons & equals) --- bin/require-analyzer | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/bin/require-analyzer b/bin/require-analyzer index 27fb714..61b5216 100755 --- a/bin/require-analyzer +++ b/bin/require-analyzer @@ -2,7 +2,6 @@ var fs = require('fs'), path = require('path'), - util = require('util'), colors = require('colors'), winston = require('winston'), argv = require('optimist').argv, @@ -24,7 +23,7 @@ var help = [ ].join('\n'); if (argv.h || argv.help) { - return util.puts(help); + return console.log(help); } // @@ -71,16 +70,16 @@ function listDependencies (pkg, msgs) { list = list.replace(/\{\s/, '{ \n') .replace(/\}/, '\n}') .replace('\033[90m', ' \033[90m') - .replace(/, /ig, ',\n ') + .replace(/, /ig, ',\n '); } else { list = list.replace(/\n\s{4}/ig, '\n '); } - winston.info(msgs.success) + winston.info(msgs.success); list.split('\n').forEach(function (line) { winston.data(line); - }) + }); } var dir = process.cwd(), @@ -96,7 +95,7 @@ pkgFile = path.join(dir, argv.f || argv.file || 'package.json'); winston.info('require-analyzer starting in ' + dir.magenta); fs.readFile(pkgFile, function (err, data) { if (err) { - if (err.errno === 34 && err.code == 'ENOENT') { + if (err.errno === 34 && err.code === 'ENOENT') { data = "{}"; } else { @@ -146,7 +145,7 @@ fs.readFile(pkgFile, function (err, data) { winston.error('Error analyzing dependencies'.red); err.message.split('\n').forEach(function (line) { winston.error(line); - }) + }); return; } }); @@ -180,8 +179,8 @@ fs.readFile(pkgFile, function (err, data) { // file in the target `dir`. // if (argv.safe) { - winston.info('did not update package.json') - return + winston.info('did not update package.json'); + return; } if (Object.keys(newpkg.dependencies).length > 0) { winston.info('Updating ' + pkgFile.magenta); @@ -194,7 +193,7 @@ fs.readFile(pkgFile, function (err, data) { } winston.info('require-analyzer updated package.json dependencies'); - }) + }); } }); }); From 5d32a739a3edc64f939b481311a9626c617cd423 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Felix=20Bo=CC=88hm?= Date: Fri, 7 Sep 2012 18:49:07 +0200 Subject: [PATCH 02/20] added a analyzer.path method that calls the appropriate function for a path as it's mainly the code of .analyze, use it there --- lib/require-analyzer.js | 93 +++++++++++++++++++++++------------------ 1 file changed, 52 insertions(+), 41 deletions(-) diff --git a/lib/require-analyzer.js b/lib/require-analyzer.js index 8c22833..294db4b 100755 --- a/lib/require-analyzer.js +++ b/lib/require-analyzer.js @@ -23,8 +23,7 @@ var analyzer = exports; // ### function analyze (options, callback) // #### @options {Object} Options to analyze against // #### @callback {function} Continuation to respond to when complete. -// Calls the appropriate `require-analyzer` method based on the result -// from `fs.stats()` on `options.target`. When dependencies are returned, +// Calls `path`. When dependencies are returned, // `npmAnalyze()` is called with `options` and the resulting object is // returned to `callback`. Also returns an event emitter which outputs // data at various stages of completion with events: @@ -35,14 +34,57 @@ var analyzer = exports; // analyzer.analyze = function (options, callback) { var emitter = new events.EventEmitter(); + + // + // let path determine what to do + // + analyzer.path(options, function (err, deps) { + if (err) { + emitter.emit('childError', err); + return callback(err); + } + + // Emit the `dependencies` event for streaming results. + emitter.emit('dependencies', deps); + + if (options.npm === false || !deps || deps.length === 0) { + return callback(null, deps); + } + + var npmEmitter = analyzer.npmAnalyze(deps, options, function (nerr, reduced, suspect) { + return callback(err || nerr, reduced, suspect); + }); + + // + // Re-emit the `search` and `reduce` events from the `npmEmitter` + // for streaming results. + // + ['search', 'reduce'].forEach(function (ev) { + npmEmitter.on(ev, function () { + var args = Array.prototype.slice.call(arguments); + args.unshift(ev); + emitter.emit.apply(emitter, args); + }); + }); + }); + + return emitter; +}; +// +// ### function path (options, callback) +// #### @options {Object} Options to analyze against +// #### @callback {function} Continuation to respond to when complete. +// Calls the appropriate `require-analyzer` method based on the result +// from `fs.stats()` on `options.target`. +// +analyzer.path = function(options, callback){ if (!options || !options.target) { // // If there are no `options` and no `options.target` property // respond with the appropriate error. // callback(new Error('options and options.target are required')); - return emitter; } // @@ -53,50 +95,19 @@ analyzer.analyze = function (options, callback) { if (err) { return callback(err); } - - var analyzeFn, rootDir; - if (stats.isDirectory()) { - analyzeFn = analyzer.dir; + else if (stats.isDirectory()) { + analyzer.dir(options, callback); } else if (stats.isFile()) { - analyzeFn = analyzer.file; + if("fileFilter" in options && !options.fileFilter(options.target)) return; + analyzer.file(options, callback); } else { - return callback(new Error(options.target + ' is not a file or a directory.')); + err = new Error(options.target + ' is not a file or a directory.'); + err.code = "UNSUPPORTED_TYPE"; + callback(err); } - - analyzeFn.call(null, options, function (err, deps) { - if (err) { - emitter.emit('childError', err); - return callback(err); - } - - // Emit the `dependencies` event for streaming results. - emitter.emit('dependencies', deps); - - if (options.npm === false || !deps || deps.length === 0) { - return callback(null, deps); - } - - var npmEmitter = analyzer.npmAnalyze(deps, options, function (nerr, reduced, suspect) { - return callback(err || nerr, reduced, suspect); - }); - - // - // Re-emit the `search` and `reduce` events from the `npmEmitter` - // for streaming results. - // - ['search', 'reduce'].forEach(function (ev) { - npmEmitter.on(ev, function () { - var args = Array.prototype.slice.call(arguments); - args.unshift(ev); - emitter.emit.apply(emitter, args); - }); - }); - }); }); - - return emitter; }; // From e432496dc2dcc2449326e8592248103c4c4fa56f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Felix=20Bo=CC=88hm?= Date: Fri, 7 Sep 2012 18:51:17 +0200 Subject: [PATCH 03/20] use .path inside .dir (replaces findit) --- lib/require-analyzer.js | 94 ++++++++++++++++++++--------------------- 1 file changed, 47 insertions(+), 47 deletions(-) diff --git a/lib/require-analyzer.js b/lib/require-analyzer.js index 294db4b..a0d23bb 100755 --- a/lib/require-analyzer.js +++ b/lib/require-analyzer.js @@ -14,8 +14,7 @@ var util = require('util'), readInstalled = require('read-installed'), detective = require('detective'), resolve = require('resolve'), - semver = require('semver'), - findit = require('findit'); + semver = require('semver'); var analyzer = exports; @@ -192,6 +191,14 @@ analyzer.npmAnalyze = function (deps, options, callback) { return emitter; }; +function filterFiles(file){ + // + // If the file is not `.js` or `.coffee` do no analyze it + // + var ext = path.extname(file); + return ext === '.js' || ext === '.coffee'; +} + // // ### function package (dir, callback) // #### @dir {string} Parent directory to analyze @@ -200,11 +207,13 @@ analyzer.npmAnalyze = function (deps, options, callback) { // running `analyzer.package()` if it exists. Otherwise attempts to run // `analyzer.file()` on all files in the source tree. // -analyzer.dir = function (options, callback) { +analyzer.dir = function (options, callback) { + var target = path.resolve(__dirname, options.target); + // // Read the target directory // - fs.readdir(options.target, function (err, files) { + fs.readdir(target, function (err, files) { if (err) { return callback(err); } @@ -213,37 +222,25 @@ analyzer.dir = function (options, callback) { // If there is a package.json in the directory // then analyze the require(s) based on `package.main` // - if ((options.target && files.indexOf(options.target) !== -1) - || files.indexOf('package.json') !== -1) { + if (files.indexOf('package.json') !== -1 || + (options.target && files.indexOf(options.target) !== -1) // TODO undestand this + ) { return analyzer.package(options, callback); } + + var remaining = files.length, + packages = {}; // // Otherwise find all files in the directory tree // and attempt to run `analyzer.file()` on each of them // in parallel. // - var files = [], - done = [], - packages = {}, - traversed = false, - target = path.resolve(__dirname, options.target), - finder = findit.find(target); - - function onRequired() { - // - // Respond to the `callback` if all files have been traversed - // and all files have been executed via `analyzer.file()` - // - if (traversed && files.length === done.length) { - callback(null, Object.keys(packages)); - } - } - - finder.on('file', function (file) { + files.forEach(function(file){ // // skip all files from "node_modules" directories - // because the checked direcotry might already be + // + // as the checked direcotry might already be // in a node_modules directory, only the relative path // is checked // @@ -251,34 +248,37 @@ analyzer.dir = function (options, callback) { if (relativePath.indexOf("node_modules") >= 0) { return; } - + // - // If the file is not `.js` or `.coffee` do no analyze it + // call analyzer.path and currate all dependencies // - var ext = path.extname(file), - clone = analyzer.merge({}, options); - - if (ext !== '.js' && ext !== '.coffee') { - return; - } - - files.push(file); - - clone.target = file; - analyzer.file(clone, function (err, deps) { - deps.forEach(function (dep) { + analyzer.path({ + __proto__: options, + target: file, + fileFilter: filterFiles + }, function(err, deps){ + if(err && err.code !== "UNSUPPORTED_TYPE"){ + // + // skip symlinks & friends + // but forward real errors + // + remaining = -1; //ensures that callback won't be called again + callback(err); + return; + } + + deps.forEach(function(dep){ packages[dep] = true; }); - - done.push(file); - onRequired(); + + // + // when all files are analyzed, call the callback + // + if(!--remaining){ + callback(null, Object.keys(packages)); + } }); }); - - finder.on('end', function () { - traversed = true; - onRequired(); - }); }); }; From b12e72dc676ff9afb7fc32884aebeaf8a6722739 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Felix=20Bo=CC=88hm?= Date: Fri, 7 Sep 2012 18:52:12 +0200 Subject: [PATCH 04/20] removed findit from dependencies --- package.json | 3 +-- test/require-analyzer-test.js | 5 +---- 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/package.json b/package.json index f739b69..b4a9a39 100644 --- a/package.json +++ b/package.json @@ -1,7 +1,7 @@ { "name": "require-analyzer", "description": "Determine dependencies for a given node.js file, directory tree, or module in code or on the command line", - "version": "0.4.1-1", + "version": "0.4.1", "author": "Nodejitsu Inc. ", "maintainers": [ "indexzero ", @@ -13,7 +13,6 @@ }, "dependencies": { "colors": "0.x.x", - "findit": "0.0.x", "optimist": "0.3.x", "semver": "1.0.x", "winston": "0.6.x", diff --git a/test/require-analyzer-test.js b/test/require-analyzer-test.js index 14dd146..2efeb6a 100644 --- a/test/require-analyzer-test.js +++ b/test/require-analyzer-test.js @@ -36,7 +36,6 @@ var rawPackages = { var libDeps = { 'colors': '0.x.x', - 'findit': '0.0.x', 'read-installed': '0.0.x', 'resolve': '0.2.x', 'optimist': '0.3.x', @@ -56,7 +55,6 @@ var libPackages = [ 'semver', 'slide', 'which', - 'findit', 'seq', 'hashish', 'traverse', @@ -67,8 +65,7 @@ var depsFromFile = [ 'read-installed', 'detective', 'resolve', - 'semver', - 'findit' + 'semver' ]; var nativeSubjects = {}; From f74cfd3e5ef8c71634936a270c512f951e4d4bda Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Felix=20Bo=CC=88hm?= Date: Fri, 7 Sep 2012 18:58:48 +0200 Subject: [PATCH 05/20] some minor changes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit • use the `in` operator more frequently (it's much nicer than `typeof foo !== "undefined"` + is more readable) • removed some garbage • added some comments --- lib/require-analyzer.js | 77 ++++++++++++++++++++++------------------- 1 file changed, 42 insertions(+), 35 deletions(-) diff --git a/lib/require-analyzer.js b/lib/require-analyzer.js index a0d23bb..b32072c 100755 --- a/lib/require-analyzer.js +++ b/lib/require-analyzer.js @@ -148,8 +148,8 @@ analyzer.npmAnalyze = function (deps, options, callback) { if (result.devDependencies && pkg in result.devDependencies) return; if (result.bundleDependencies && pkg in result.bundleDependencies) return; if (!Array.isArray(deps)) { - if (deps[pkg] === '*' || typeof deps[pkg] === 'undefined') { - pkgs[pkg] = result.dependencies[pkg] + if (deps[pkg] === '*' || !(pkg in deps) ) { + pkgs[pkg] = pkg in result.dependencies ? result.dependencies[pkg]['version'] : deps[pkg]; } @@ -168,13 +168,13 @@ analyzer.npmAnalyze = function (deps, options, callback) { return callback(null, pkgs); } - var reduced = analyzer.merge({}, pkgs), + var reduced = analyzer.clone(pkgs), suspect = {}; Object.keys(deps).forEach(function (dep) { - if (pkgs[dep] && pkgs[dep].dependencies) { + if (dep in pkgs && pkgs[dep].dependencies) { Object.keys(pkgs[dep].dependencies).forEach(function (cdep) { - if (reduced[cdep]) { + if (cdep in reduced) { suspect[cdep] = pkgs[cdep]; delete reduced[cdep]; } @@ -345,7 +345,7 @@ analyzer.package = function (options, callback) { }); } - var scripts = options.hasOwnProperty('scripts') ? options.scripts : ["test","prestart"]; + var scripts = 'scripts' in options ? options.scripts : ["test","prestart"]; scripts = scripts.map(function (item) { return pkg.scripts && pkg.scripts[item]; @@ -387,7 +387,7 @@ analyzer.package = function (options, callback) { } function checkFile(file) { - exists(file, fileExists) + exists(file, fileExists); } function fileExists(exists) { @@ -415,8 +415,8 @@ analyzer.package = function (options, callback) { } // add logic to default to app.js or server.js for main if main is not present. - if (typeof pkg.main === 'undefined' || pkg.main === '') { - var files = ["app.js", "server.js", "index.js"] + if ( !("main" in pkg) || pkg.main === '') { + var files = ["app.js", "server.js", "index.js"]; setMain(files, pkg, newoptions, setTarget); } else { @@ -576,24 +576,24 @@ analyzer.file = function(options, callback){ analyzer.findModulesDir = function (target, callback) { fs.stat(target, function (err, stats) { if (err) { - return callback(err); + callback(err); } - if (stats.isDirectory()) { - return fs.readdir(target, function (err, files) { + else if (stats.isDirectory()) { + fs.readdir(target, function (err, files) { if (err) { - return callback(err); + callback(err); } - - if (files.indexOf('node_modules') !== -1 || files.indexOf('package.json') !== -1) { - return callback(null, target); + //TODO behave differently when a node_modules dir is present + else if (files.indexOf('node_modules') !== -1 || files.indexOf('package.json') !== -1) { + callback(null, target); } else { - return callback(null, target); + callback(null, target); } }); } else if (stats.isFile()) { - return analyzer.findModulesDir(path.dirname(target), callback); + analyzer.findModulesDir(path.dirname(target), callback); } }); }; @@ -604,11 +604,13 @@ analyzer.findModulesDir = function (target, callback) { // Merges all properties in `arg1 ... argn` // into the `target` object. // +// TODO remove this as it isn't used anymore +// analyzer.merge = function (target) { var objs = Array.prototype.slice.call(arguments, 1); objs.forEach(function (o) { Object.keys(o).forEach(function (attr) { - if (! o.__lookupGetter__(attr)) { + if ( !("get" in Object.getOwnPropertyDescriptor(o, attr)) ) { target[attr] = o[attr]; } }); @@ -654,7 +656,7 @@ analyzer.extractVersions = function (dependencies) { parse = semver.expressions.parse.exec(raw.trim()), version = parse ? parse.slice(1) : raw, build = version ? version[3] || version[4] : null; - if (!/^[v\d]+/.test(raw)) { + if (!/^[v\d]/.test(raw)) { all[pkg] = raw; } else if (typeof version === 'string') { @@ -682,17 +684,15 @@ analyzer.extractVersions = function (dependencies) { // updated: { /* Union of updated / current with new versions */ } // } // +var cleanVersion = /\<|\>|\=|\s/ig; + analyzer.updates = function (current, updated) { var updates = { - added: {}, + added: !current && updated || {}, updated: {} }; - if (!current) { - updates.updated = updated || {}; - return updates; - } - else if (!updated) { + if (!current || !updated) { return updates; } @@ -700,7 +700,7 @@ analyzer.updates = function (current, updated) { // Get the list of all added dependencies // Object.keys(updated).filter(function (key) { - return !current[key]; + return !(key in current); }).forEach(function (key) { updates.added[key] = updated[key]; }); @@ -709,17 +709,17 @@ analyzer.updates = function (current, updated) { // Get the list of all dependencies that have been updated // Object.keys(updated).filter(function (key) { - if (!current[key]) { + if ( !(key in current) ) { return false; } - var left = updated[key].replace(/\<|\>|\=|\s/ig, ''), - right = current[key].replace(/\<|\>|\=|\s/ig, ''); + var left = updated[key].replace(cleanVersion, ''), + right = current[key].replace(cleanVersion, ''); return semver.gt(left, right); }).forEach(function (key) { updates.updated[key] = updated[key]; - }) + }); return updates; }; @@ -730,16 +730,23 @@ analyzer.updates = function (current, updated) { // Check if `module` is a native module (like `net` or `tty`). // // TODO use the resolve module for this +// (faster & doesn't depend on the node version) // analyzer.isNative = function (module) { try { - return require.resolve(module) == module; + return require.resolve(module) === module; } catch (err) { return false; } }; +// +// ### function resolve (file, base) +// #### @file {string} filename +// #### @base {string} the root from which the file should be searched +// Check if `module` is a native module (like `net` or `tty`). +// analyzer.resolve = function(file, base){ try { return resolve.sync(file, { @@ -755,12 +762,12 @@ function mergeDependencies(err, deps, pkgDeps, devDeps, bndlDeps, callback) { function removeDevDeps(deps) { var obj = analyzer.clone(deps), dep; for (dep in devDeps) { - if (typeof obj[dep] !== 'undefined') { + if (dep in obj) { delete obj[dep]; } } for (dep in bndlDeps) { - if (typeof obj[dep] !== 'undefined') { + if (dep in obj) { delete obj[dep]; } } @@ -792,7 +799,7 @@ function mergeDependencies(err, deps, pkgDeps, devDeps, bndlDeps, callback) { }); Object.keys(pkgDeps).forEach(function (d) { - if (typeof merged[d] === 'undefined') { + if ( !(d in merged[d]) ) { merged[d] = pkgDeps[d]; } }); From e1aa6b0cac6297751f58f4d33d0f8cd1bb5e0872 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Felix=20Bo=CC=88hm?= Date: Fri, 7 Sep 2012 19:17:04 +0200 Subject: [PATCH 06/20] removed two unnecessary functions in L54, err would have always be falsey (or the function had returned earlier) --- lib/require-analyzer.js | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/lib/require-analyzer.js b/lib/require-analyzer.js index b32072c..d9c3799 100755 --- a/lib/require-analyzer.js +++ b/lib/require-analyzer.js @@ -50,9 +50,7 @@ analyzer.analyze = function (options, callback) { return callback(null, deps); } - var npmEmitter = analyzer.npmAnalyze(deps, options, function (nerr, reduced, suspect) { - return callback(err || nerr, reduced, suspect); - }); + var npmEmitter = analyzer.npmAnalyze(deps, options, callback); // // Re-emit the `search` and `reduce` events from the `npmEmitter` @@ -132,7 +130,7 @@ analyzer.npmAnalyze = function (deps, options, callback) { } // - // Analyze dependencies by searching for all installed locally via npm. + // Analyze dependencies by searching for all installed locally via read-installed. // Then see if it depends on any other dependencies that are in the // list so those dependencies may be removed (only if `options.reduce` is set). // @@ -141,6 +139,7 @@ analyzer.npmAnalyze = function (deps, options, callback) { return callback(err); } else if (!result || !result.dependencies || Object.keys(result.dependencies).length === 0) { + // When no dependencies were found, return what we got return callback(null, deps); } @@ -349,9 +348,7 @@ analyzer.package = function (options, callback) { scripts = scripts.map(function (item) { return pkg.scripts && pkg.scripts[item]; - }).filter(function (item) { - return !!item; - }); + }).filter(Boolean); if (scripts) { scripts.forEach(function analyzeScript(script) { From f08e5fcf84fe3086943feb93baa471a00c39966c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Felix=20Bo=CC=88hm?= Date: Fri, 7 Sep 2012 20:21:51 +0200 Subject: [PATCH 07/20] [fix] still accessed property before checking it with `in` --- lib/require-analyzer.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/require-analyzer.js b/lib/require-analyzer.js index d9c3799..a875709 100755 --- a/lib/require-analyzer.js +++ b/lib/require-analyzer.js @@ -796,7 +796,7 @@ function mergeDependencies(err, deps, pkgDeps, devDeps, bndlDeps, callback) { }); Object.keys(pkgDeps).forEach(function (d) { - if ( !(d in merged[d]) ) { + if ( !(d in merged) ) { merged[d] = pkgDeps[d]; } }); From 55faa2957f5f28040d5a15dea6b0ef51f2f721af Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Felix=20Bo=CC=88hm?= Date: Fri, 7 Sep 2012 20:25:48 +0200 Subject: [PATCH 08/20] [fix] passed filenames instead of paths to .path --- lib/require-analyzer.js | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/lib/require-analyzer.js b/lib/require-analyzer.js index a875709..d210e2d 100755 --- a/lib/require-analyzer.js +++ b/lib/require-analyzer.js @@ -239,21 +239,14 @@ analyzer.dir = function (options, callback) { // // skip all files from "node_modules" directories // - // as the checked direcotry might already be - // in a node_modules directory, only the relative path - // is checked - // - var relativePath = path.relative(target, file); - if (relativePath.indexOf("node_modules") >= 0) { - return; - } + if(file === "node_modules") return; // // call analyzer.path and currate all dependencies // analyzer.path({ __proto__: options, - target: file, + target: path.join(target, file), fileFilter: filterFiles }, function(err, deps){ if(err && err.code !== "UNSUPPORTED_TYPE"){ From a7c35d1d8710efdec2c884f7448c0d19b05580d7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Felix=20Bo=CC=88hm?= Date: Fri, 7 Sep 2012 20:32:57 +0200 Subject: [PATCH 09/20] forward errors from prerunners in runAnalyze --- test/example-apps-test.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/example-apps-test.js b/test/example-apps-test.js index 01d6386..391f39a 100644 --- a/test/example-apps-test.js +++ b/test/example-apps-test.js @@ -17,7 +17,8 @@ function dependencies (file, prerunner) { return function () { var that = this; - function runAnalyze () { + function runAnalyze (err) { + if(err) return that.callback(err); analyzer.analyze({ target: path.join(__dirname, file) }, From d9c45ebb3d4b5558d44941bfb3f19fac0798d668 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Felix=20Bo=CC=88hm?= Date: Sat, 8 Sep 2012 11:33:22 +0200 Subject: [PATCH 10/20] use Function#bind when it's appropriate, use single quotes --- lib/require-analyzer.js | 30 +++++++++++++----------------- 1 file changed, 13 insertions(+), 17 deletions(-) diff --git a/lib/require-analyzer.js b/lib/require-analyzer.js index d210e2d..6556db7 100755 --- a/lib/require-analyzer.js +++ b/lib/require-analyzer.js @@ -57,11 +57,7 @@ analyzer.analyze = function (options, callback) { // for streaming results. // ['search', 'reduce'].forEach(function (ev) { - npmEmitter.on(ev, function () { - var args = Array.prototype.slice.call(arguments); - args.unshift(ev); - emitter.emit.apply(emitter, args); - }); + npmEmitter.on(ev, emitter.emit.bind(emitter, ev)); }); }); @@ -96,12 +92,12 @@ analyzer.path = function(options, callback){ analyzer.dir(options, callback); } else if (stats.isFile()) { - if("fileFilter" in options && !options.fileFilter(options.target)) return; + if('fileFilter' in options && !options.fileFilter(options.target)) return; analyzer.file(options, callback); } else { err = new Error(options.target + ' is not a file or a directory.'); - err.code = "UNSUPPORTED_TYPE"; + err.code = 'UNSUPPORTED_TYPE'; callback(err); } }); @@ -237,9 +233,9 @@ analyzer.dir = function (options, callback) { // files.forEach(function(file){ // - // skip all files from "node_modules" directories + // skip all files from 'node_modules' directories // - if(file === "node_modules") return; + if(file === 'node_modules') return; // // call analyzer.path and currate all dependencies @@ -249,7 +245,7 @@ analyzer.dir = function (options, callback) { target: path.join(target, file), fileFilter: filterFiles }, function(err, deps){ - if(err && err.code !== "UNSUPPORTED_TYPE"){ + if(err && err.code !== 'UNSUPPORTED_TYPE'){ // // skip symlinks & friends // but forward real errors @@ -337,7 +333,7 @@ analyzer.package = function (options, callback) { }); } - var scripts = 'scripts' in options ? options.scripts : ["test","prestart"]; + var scripts = 'scripts' in options ? options.scripts : ['test','prestart']; scripts = scripts.map(function (item) { return pkg.scripts && pkg.scripts[item]; @@ -405,8 +401,8 @@ analyzer.package = function (options, callback) { } // add logic to default to app.js or server.js for main if main is not present. - if ( !("main" in pkg) || pkg.main === '') { - var files = ["app.js", "server.js", "index.js"]; + if ( !('main' in pkg) || pkg.main === '') { + var files = ['app.js', 'server.js', 'index.js']; setMain(files, pkg, newoptions, setTarget); } else { @@ -478,12 +474,12 @@ function spawnWorker (options, callback) { deps.send(options.target); - deps.on("message", function(data){ + deps.on('message', function(data){ switch(data.type){ - case "load": + case 'load': packages[data.msg] = true; break; - case "error": + case 'error': errs.push(data.msg); } }); @@ -600,7 +596,7 @@ analyzer.merge = function (target) { var objs = Array.prototype.slice.call(arguments, 1); objs.forEach(function (o) { Object.keys(o).forEach(function (attr) { - if ( !("get" in Object.getOwnPropertyDescriptor(o, attr)) ) { + if ( !('get' in Object.getOwnPropertyDescriptor(o, attr)) ) { target[attr] = o[attr]; } }); From 48a34e0a195bf0a169786924218e2275febe5c0d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Felix=20Bo=CC=88hm?= Date: Sat, 8 Sep 2012 11:34:09 +0200 Subject: [PATCH 11/20] Revert "forward errors from prerunners in runAnalyze" This reverts commit a7c35d1d8710efdec2c884f7448c0d19b05580d7. --- test/example-apps-test.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/test/example-apps-test.js b/test/example-apps-test.js index 391f39a..01d6386 100644 --- a/test/example-apps-test.js +++ b/test/example-apps-test.js @@ -17,8 +17,7 @@ function dependencies (file, prerunner) { return function () { var that = this; - function runAnalyze (err) { - if(err) return that.callback(err); + function runAnalyze () { analyzer.analyze({ target: path.join(__dirname, file) }, From 7ede4628e5310412e1b2b3425ce4d9aeb4d352c6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Felix=20Bo=CC=88hm?= Date: Sat, 8 Sep 2012 18:12:55 +0200 Subject: [PATCH 12/20] always return an object in npmAnalyze also moved the creation of the find-dependencies path to the surrounding scope --- lib/require-analyzer.js | 23 ++++++++++++++++++----- 1 file changed, 18 insertions(+), 5 deletions(-) diff --git a/lib/require-analyzer.js b/lib/require-analyzer.js index 6556db7..e0c7a40 100755 --- a/lib/require-analyzer.js +++ b/lib/require-analyzer.js @@ -136,7 +136,15 @@ analyzer.npmAnalyze = function (deps, options, callback) { } else if (!result || !result.dependencies || Object.keys(result.dependencies).length === 0) { // When no dependencies were found, return what we got - return callback(null, deps); + if(Array.isArray(deps)){ + return callback(null, deps.reduce(function(obj, prop){ + obj[prop] = "*"; + return obj; + }, {})); + } + else { + return callback(null, deps); + } } Object.keys(result.dependencies).forEach(function (pkg) { @@ -463,6 +471,8 @@ function analyzeFile (options, callback) { }); } +var findDepsPath = path.join(__dirname, '..', 'bin', 'find-dependencies'); + function spawnWorker (options, callback) { // // Spawn the `find-dependencies` bin helper to ensure that we are able to @@ -470,7 +480,7 @@ function spawnWorker (options, callback) { // var packages = options.packages, errs = options.errors, - deps = fork(path.join(__dirname, '..', 'bin', 'find-dependencies'), [options.target], {silent: true}); + deps = fork(findDepsPath, [options.target], {silent: true}); deps.send(options.target); @@ -503,7 +513,6 @@ function spawnWorker (options, callback) { // Remove the timeout now that we have exited. // clearTimeout(timeoutId); - callback(); }); } @@ -518,8 +527,9 @@ function spawnWorker (options, callback) { // analyzer.file = function(options, callback){ - options.packages = options.packages || {}; - options.errors = options.errors || []; + if(!options.packages) options.packages = {}; + if(!options.errors) options.errors = []; + analyzeFile(options, function(err){ if(options.errors.length > 0){ callback(options.errors); //TODO call with real error object @@ -581,6 +591,9 @@ analyzer.findModulesDir = function (target, callback) { else if (stats.isFile()) { analyzer.findModulesDir(path.dirname(target), callback); } + else { + callback(new Error(target + ' is not a file or a directory.')); + } }); }; From b3e2bb92c3b42f300cd9aec499e5f5bfdffb3b01 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Felix=20Bo=CC=88hm?= Date: Sat, 8 Sep 2012 18:22:26 +0200 Subject: [PATCH 13/20] [fix] move upwards in findModulesDir this is the functionality that is described in the comments, but wasn't implemented (yet) --- lib/require-analyzer.js | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/lib/require-analyzer.js b/lib/require-analyzer.js index e0c7a40..10fc2d2 100755 --- a/lib/require-analyzer.js +++ b/lib/require-analyzer.js @@ -579,12 +579,14 @@ analyzer.findModulesDir = function (target, callback) { if (err) { callback(err); } - //TODO behave differently when a node_modules dir is present else if (files.indexOf('node_modules') !== -1 || files.indexOf('package.json') !== -1) { callback(null, target); } + else if (target === (target = path.dirname(target))){ + callback(new Error('Couldn\'t find a node_modules directory.')); + } else { - callback(null, target); + analyzer.findModulesDir(target, callback); } }); } From 49b2d4464c1d823825ce7d0035d76324932c3f99 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Felix=20Bo=CC=88hm?= Date: Sat, 8 Sep 2012 18:57:07 +0200 Subject: [PATCH 14/20] removed senseless check in analyzer.dir a folder should never contain a file that has the folder's path as it's name --- lib/require-analyzer.js | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/lib/require-analyzer.js b/lib/require-analyzer.js index 10fc2d2..a1b38bd 100755 --- a/lib/require-analyzer.js +++ b/lib/require-analyzer.js @@ -225,9 +225,7 @@ analyzer.dir = function (options, callback) { // If there is a package.json in the directory // then analyze the require(s) based on `package.main` // - if (files.indexOf('package.json') !== -1 || - (options.target && files.indexOf(options.target) !== -1) // TODO undestand this - ) { + if (files.indexOf('package.json') !== -1) { return analyzer.package(options, callback); } From 16f22f9059485db65a28739c9adb2460d4c8be62 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Felix=20Bo=CC=88hm?= Date: Sat, 8 Sep 2012 19:08:04 +0200 Subject: [PATCH 15/20] added analyzer.findNextDir function the last change broke most tests, so .findNextDir is what's actually wanted for .npmAnalyze --- lib/require-analyzer.js | 51 +++++++++++++++++++++++++---------------- 1 file changed, 31 insertions(+), 20 deletions(-) diff --git a/lib/require-analyzer.js b/lib/require-analyzer.js index a1b38bd..8cc3c9a 100755 --- a/lib/require-analyzer.js +++ b/lib/require-analyzer.js @@ -120,7 +120,7 @@ analyzer.npmAnalyze = function (deps, options, callback) { return callback(); } - analyzer.findModulesDir(options.target, function (err, root) { + analyzer.findNextDir(options.target, function (err, root) { if (err) { return callback(err); } @@ -562,31 +562,17 @@ analyzer.file = function(options, callback){ }; // -// ### function findModulesDir (target) -// #### @target {string} The directory (or file) to search up from -// Searches up from the specified `target` until it finds a directory which contains -// a folder called `node_modules` +// ### function findNextDir (target) +// #### @target {string} The path to search up from +// Searches up from the specified `target` until it finds a directory // -analyzer.findModulesDir = function (target, callback) { +analyzer.findNextDir = function(target, callback) { fs.stat(target, function (err, stats) { if (err) { callback(err); } else if (stats.isDirectory()) { - fs.readdir(target, function (err, files) { - if (err) { - callback(err); - } - else if (files.indexOf('node_modules') !== -1 || files.indexOf('package.json') !== -1) { - callback(null, target); - } - else if (target === (target = path.dirname(target))){ - callback(new Error('Couldn\'t find a node_modules directory.')); - } - else { - analyzer.findModulesDir(target, callback); - } - }); + callback(null, target); } else if (stats.isFile()) { analyzer.findModulesDir(path.dirname(target), callback); @@ -597,6 +583,31 @@ analyzer.findModulesDir = function (target, callback) { }); }; +// +// ### function findModulesDir (target) +// #### @target {string} The directory (or file) to search up from +// Searches up from the specified `target` until it finds a directory which contains +// a folder called `node_modules` +// +analyzer.findModulesDir = function (target, callback) { + analyzer.findNextDir(target, function(err, dir){ + fs.readdir(target, function (err, files) { + if (err) { + callback(err); + } + else if (files.indexOf('node_modules') !== -1 || files.indexOf('package.json') !== -1) { + callback(null, target); + } + else if (target === (target = path.dirname(target))){ + callback(new Error('Couldn\'t find a node_modules directory.')); + } + else { + analyzer.findModulesDir(target, callback); + } + }); + }); +}; + // // ### function (target [arg1, arg2, ...]) // #### @target {Object} Object to merge into From eeebe3a1605c877fa4d95c416ad71844da19a9ed Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Felix=20Bo=CC=88hm?= Date: Sat, 8 Sep 2012 19:43:24 +0200 Subject: [PATCH 16/20] set 1 as the maxDepth of read-installed we don't use more, so don't do anything fancy --- lib/require-analyzer.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/require-analyzer.js b/lib/require-analyzer.js index 8cc3c9a..3fde360 100755 --- a/lib/require-analyzer.js +++ b/lib/require-analyzer.js @@ -130,7 +130,7 @@ analyzer.npmAnalyze = function (deps, options, callback) { // Then see if it depends on any other dependencies that are in the // list so those dependencies may be removed (only if `options.reduce` is set). // - readInstalled(root, function (err, result) { + readInstalled(root, 1, function (err, result) { if (err) { return callback(err); } From 873a6412f7e2a29a134c6dc31d9d4ab4c8c13abc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Felix=20Bo=CC=88hm?= Date: Sat, 8 Sep 2012 19:48:17 +0200 Subject: [PATCH 17/20] [fix] call callback when a node_modules folder is present --- lib/require-analyzer.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/require-analyzer.js b/lib/require-analyzer.js index 3fde360..e3b1196 100755 --- a/lib/require-analyzer.js +++ b/lib/require-analyzer.js @@ -231,7 +231,7 @@ analyzer.dir = function (options, callback) { var remaining = files.length, packages = {}; - + // // Otherwise find all files in the directory tree // and attempt to run `analyzer.file()` on each of them @@ -241,7 +241,7 @@ analyzer.dir = function (options, callback) { // // skip all files from 'node_modules' directories // - if(file === 'node_modules') return; + if(file === 'node_modules') return remaining--; // // call analyzer.path and currate all dependencies From e169c9ebe6a3eda1f1bef5badc86aed3a9ce73cb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Felix=20Bo=CC=88hm?= Date: Sat, 8 Sep 2012 23:35:22 +0200 Subject: [PATCH 18/20] simplified mergeDependencies & .package MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit • removed scanning scripts as they weren't implemented properly (they just looped a couple of times without doing anything) • simplified signature of mergeDependencies • return errors in .package • also updated some comments --- lib/require-analyzer.js | 80 ++++++++++++++--------------------------- 1 file changed, 26 insertions(+), 54 deletions(-) diff --git a/lib/require-analyzer.js b/lib/require-analyzer.js index e3b1196..dcb3821 100755 --- a/lib/require-analyzer.js +++ b/lib/require-analyzer.js @@ -108,7 +108,7 @@ analyzer.path = function(options, callback){ // #### @deps {Array} List of dependencies to analyze. // #### @options {Object} Set of options to analyze with. // #### @callback {function} Continuation to respond to when complete. -// Analyzes the list of dependencies using `npm`, consumes the options: +// Analyzes the list of dependencies using `read-installed`, consumes the options: // // options.reduce: Will remove deps consumed by sibling deps // @@ -284,10 +284,6 @@ analyzer.dir = function (options, callback) { // the require statements in the script located at `package.main` // analyzer.package = function (options, callback) { - var deps = {}, - pkgDeps = {}, - devDeps = {}, - bdlDeps = {}; // // Attempt to read the package.json in the current directory // @@ -298,9 +294,6 @@ analyzer.package = function (options, callback) { // Attempt to read the package.json data. // pkg = JSON.parse(pkg.toString()); - pkgDeps = pkg.dependencies; - devDeps = pkg.devDependencies; - bdlDeps = pkg.bundleDependencies; } catch (e) { return callback(e); @@ -314,7 +307,6 @@ analyzer.package = function (options, callback) { // // Analyze the require(s) based on: // - the `main` property of the package.json - // - the scripts in the options that relate to package.json // - the default file if no package.json exists // var todo = 0, @@ -323,7 +315,8 @@ analyzer.package = function (options, callback) { function dequeue(err) { todo--; if (todo === 0) { - mergeDependencies(err, _deps, pkgDeps, devDeps, bdlDeps, callback); + if(err) callback(err); + else mergeDependencies(_deps, pkg, callback); } } @@ -332,39 +325,13 @@ analyzer.package = function (options, callback) { analyzer.file(options, function (err, deps) { _deps = _deps.concat(deps.filter(function (d) { - return _deps.indexOf(d) === -1 && d !== pkg.name; + return d !== pkg.name && _deps.indexOf(d) === -1; })); dequeue(err); }); } - var scripts = 'scripts' in options ? options.scripts : ['test','prestart']; - - scripts = scripts.map(function (item) { - return pkg.scripts && pkg.scripts[item]; - }).filter(Boolean); - - if (scripts) { - scripts.forEach(function analyzeScript(script) { - if (!script) { - return; - } - - var newoptions = analyzer.clone(options); - try { - newoptions.target = require.resolve(path.join(newoptions.target, path.normalize(pkg.main || '/'))); - } - catch (e) { - todo = 1; - deps = null; - dequeue(e); - } - - processOptions(newoptions); - }); - } - var newoptions = analyzer.clone(options); function setMain(files, pkg, newoptions, callback) { @@ -395,21 +362,20 @@ analyzer.package = function (options, callback) { } function setTarget(pkg, newoptions) { - try { - newoptions.target = require.resolve(path.join(newoptions.target, path.normalize(pkg.main || '/'))); - } - catch (e) { + var newPath = path.join(newoptions.target, pkg.main ? path.normalize(pkg.main) : '/'), + newTarget = analyzer.resolve(newPath); + + if (newTarget === false) { todo = 1; deps = null; - dequeue(e); + dequeue(new Error('Couldn\'t resolve path ' + newPath)); } return processOptions(newoptions); } // add logic to default to app.js or server.js for main if main is not present. if ( !('main' in pkg) || pkg.main === '') { - var files = ['app.js', 'server.js', 'index.js']; - setMain(files, pkg, newoptions, setTarget); + setMain(['app.js', 'server.js', 'index.js'], pkg, newoptions, setTarget); } else { setTarget(pkg, newoptions); @@ -596,6 +562,7 @@ analyzer.findModulesDir = function (target, callback) { callback(err); } else if (files.indexOf('node_modules') !== -1 || files.indexOf('package.json') !== -1) { + //TODO ensure it's actually a directory/file callback(null, target); } else if (target === (target = path.dirname(target))){ @@ -768,17 +735,25 @@ analyzer.resolve = function(file, base){ } }; -function mergeDependencies(err, deps, pkgDeps, devDeps, bndlDeps, callback) { +function mergeDependencies(deps, pkg, callback) { + var pkgDeps = pkg.dependencies; + function removeDevDeps(deps) { var obj = analyzer.clone(deps), dep; - for (dep in devDeps) { - if (dep in obj) { - delete obj[dep]; + + if('devDependencies' in pkg){ + for (dep in pkg.devDependencies) { + if (dep in obj) { + delete obj[dep]; + } } } - for (dep in bndlDeps) { - if (dep in obj) { - delete obj[dep]; + + if('bundleDependencies' in pkg){ + for (dep in pkg.bundleDependencies) { + if (dep in obj) { + delete obj[dep]; + } } } return obj; @@ -786,9 +761,6 @@ function mergeDependencies(err, deps, pkgDeps, devDeps, bndlDeps, callback) { var merged = {}; - if (err) { - return callback(err); - } if (!Array.isArray(deps)) { if (typeof deps === 'undefined' || Object.keys(deps).length === 0) { From 4968886663ed2522899608dd197ed197cb8f202f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Felix=20Bo=CC=88hm?= Date: Sat, 8 Sep 2012 23:39:55 +0200 Subject: [PATCH 19/20] removed some garbage in .package --- lib/require-analyzer.js | 28 ++++++++++------------------ 1 file changed, 10 insertions(+), 18 deletions(-) diff --git a/lib/require-analyzer.js b/lib/require-analyzer.js index dcb3821..e9d2774 100755 --- a/lib/require-analyzer.js +++ b/lib/require-analyzer.js @@ -335,30 +335,23 @@ analyzer.package = function (options, callback) { var newoptions = analyzer.clone(options); function setMain(files, pkg, newoptions, callback) { - var file = null; - function nextFile() { - file = files.shift(); - if (typeof file === 'undefined') { + if (!files.length) { return callback(pkg, newoptions); } - checkFile(file); - } - function checkFile(file) { - exists(file, fileExists); - } - - function fileExists(exists) { - if (exists) { - pkg.main = file; - return callback(pkg, newoptions); - } - nextFile(); + var file = files.shift(); + + exists(file, function(exists){ + if (exists) { + pkg.main = file; + callback(pkg, newoptions); + } + else nextFile(); + }); } nextFile(); - return; } function setTarget(pkg, newoptions) { @@ -367,7 +360,6 @@ analyzer.package = function (options, callback) { if (newTarget === false) { todo = 1; - deps = null; dequeue(new Error('Couldn\'t resolve path ' + newPath)); } return processOptions(newoptions); From 2352c317b56cc98eed54f38604935f052ce88168 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Felix=20Bo=CC=88hm?= Date: Sat, 8 Sep 2012 23:48:55 +0200 Subject: [PATCH 20/20] [fix] call .findNextDir recursively --- lib/require-analyzer.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/require-analyzer.js b/lib/require-analyzer.js index e9d2774..24f09a0 100755 --- a/lib/require-analyzer.js +++ b/lib/require-analyzer.js @@ -533,7 +533,7 @@ analyzer.findNextDir = function(target, callback) { callback(null, target); } else if (stats.isFile()) { - analyzer.findModulesDir(path.dirname(target), callback); + analyzer.findNextDir(path.dirname(target), callback); } else { callback(new Error(target + ' is not a file or a directory.'));