Use detective for parsing js requires

[stephenmathieson]

detective provides a much more reliable way to locate require() statements than a regexp. This now allows us to parse requires prefixed with // that aren't a comment (see duojs/duo#305).

A test demonstrating the issue has been included, as well as some minor whitespace cleanup in the test file itself.

Also, some unrelated but necessary stuff (Makefile magic, .gitignore, "repository" in package.json...) handled in separate commits. Will back them out if you like.

[matthewmueller]

Hmm, I'd like to run a benchmark of these changes. When I was developing this, I explicitly avoided using detective because it was slow for big projects like jQuery.

We can fix that bug pretty simply by just ignore comment syntax that aren't in strings, or even easier is just ensure that // is only prefixed by \s*. This may lead to some inline commented out requires getting added, but I don't think that's a big deal.

[stephenmathieson]

yeah i do imagine it's quite a bit slower. i'll put a little benchmark script together and post the results

[stephenmathieson]

interestingly enough, there's no worth-while difference:

here's my "bench.js":

var fs = require('fs');
var read = fs.readFileSync;
var deps = require('./');

var fixture = './test/fixtures/test.js';
var js = read(fixture).toString();

var n = process.env.BENCHMARK || 1000000;

console.log('benchmarking %d iterations', n);

console.time('file-deps');
while (--n) deps(js);
console.timeEnd('file-deps');

and the results:

stephenmathieson at MBP in ~/repos/node/file-deps on fix/js-slash-requires*
$ node bench
benchmarking 1000000 iterations
file-deps: 99ms
stephenmathieson at MBP in ~/repos/node/file-deps on fix/js-slash-requires*
$ git checkout master
Switched to branch 'master'
Your branch is up-to-date with 'origin/master'.
stephenmathieson at MBP in ~/repos/node/file-deps on master*
$ node bench
benchmarking 1000000 iterations
file-deps: 100ms
stephenmathieson at MBP in ~/repos/node/file-deps on master*

now with replacing require()s, my implementation is slightly faster:

stephenmathieson at MBP in ~/repos/node/file-deps on fix/js-slash-requires*
$ node bench
benchmarking 1000000 iterations
file-deps: 602ms
stephenmathieson at MBP in ~/repos/node/file-deps on fix/js-slash-requires*
$ git checkout master
Switched to branch 'master'
Your branch is up-to-date with 'origin/master'.
stephenmathieson at MBP in ~/repos/node/file-deps on master*
$ node bench
benchmarking 1000000 iterations
file-deps: 651ms

btw: modified "bench.js" to include:

// while (--n) deps(js);
while (--n) deps(js, function () { return 'foo' });

[stephenmathieson]

@MatthewMueller thoughts?

[matthewmueller]

oops, sorry for the late response on this. these are very surprising results. did you bench a big lib like jQuery?

[stephenmathieson]

bench a big lib like jQuery

nope, was lazy and just used files already in the repo. i'll try it this afternoon though

[stephenmathieson]

@MatthewMueller just finally tried jquery:

bench.js

var fs = require('fs');
var read = fs.readFileSync;
var deps = require('./');

var fixture = './node_modules/jquery/dist/jquery.js';
var js = read(fixture).toString();

var n = process.env.BENCHMARK || 1000000;

console.log('benchmarking %d iterations', n);

console.time('file-deps');
while (--n) deps(js);
console.timeEnd('file-deps');

benchmarks

$ npm install jquery
jquery@2.1.1 node_modules/jquery
stephenmathieson at MBP in ~/repos/node/file-deps on fix/js-slash-requires*
$ node bench
benchmarking 1000000 iterations
file-deps: 101ms
stephenmathieson at MBP in ~/repos/node/file-deps on fix/js-slash-requires*
$ git checkout master
Switched to branch 'master'
Your branch is up-to-date with 'origin/master'.
stephenmathieson at MBP in ~/repos/node/file-deps on master*
$ node bench
benchmarking 1000000 iterations
file-deps: 101ms

the difference is so trivial that console.time cannot calculate it..

[matthewmueller]

sorry for the late merge, just ran into an issue where the current implementation messed up, but this worked like a charm.

thanks!