Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 20 additions & 0 deletions index.js
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,17 @@ module.exports = function requireDir(dir, opts) {
}

if (FS.statSync(path).isDirectory()) {

// If we're not recursing through the files then we check to see
// if the folder is a valid node module

// Checks to see if there is an index file and returns the extension
var modulePath = getModulePath(path);

if (modulePath && !opts.recurse) {
filesMinusDirs[file + Path.extname(modulePath)] = modulePath;
}

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(Assuming for now we keep your design that this should be the default behavior) Should this happen only if opts.recurse isn't specified?

Put another way: what should happen when opts.recurse is specified? Currently, we'd require the dir normally, and then that would get overwritten on recurse, is that right? So harmless, but inefficient?

Can we add a test case for what should happen when opts.recurse is specified and a (sub)directory is a module? Thanks!

if (opts.recurse) {
if (base === 'node_modules') {
continue;
Expand Down Expand Up @@ -100,6 +111,7 @@ module.exports = function requireDir(dir, opts) {

// if a file exists with this extension, we'll require() it:
var file = base + ext;

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Accidental whitespace change I'm guessing.

var path = filesMinusDirs[file];

if (path) {
Expand Down Expand Up @@ -134,6 +146,14 @@ module.exports = function requireDir(dir, opts) {
return map;
};

function getModulePath(path) {
try {
return require.resolve(path);
} catch (_) {
return false;
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor: let's catch (err) and return null instead of false.

}
}

function toCamelCase(str) {
return str.replace(/[_-][a-z]/ig, function (s) {
return s.substring(1).toUpperCase();
Expand Down
8 changes: 8 additions & 0 deletions test/directoryAsModule.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
var assert = require('assert');
var requireDir = require('..');

assert.deepEqual(requireDir('./directoryAsModule'), {
a: 'a', b: 'b'
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor: let's keep to the code style of having these object keys be on their own lines.

});

console.log('Automatic Index tests passed')
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor: let's have this console.log message be consistent with the feature & test name: "Directory as module tests passed".

1 change: 1 addition & 0 deletions test/directoryAsModule/a.js
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
module.exports = 'a';
1 change: 1 addition & 0 deletions test/directoryAsModule/b/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
module.exports = 'b'
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we add a second file to this directory, to explicitly test (and convey) that it isn't getting required?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done :)

1 change: 1 addition & 0 deletions test/directoryAsModule/c/hello.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Not a module