Added check to see if folder is a valid node module

[pezza3434]

Added check to see if a folder is a node module as per require specs.

https://nodejs.org/api/modules.html#modules_folders_as_modules

@aseemk

[AlexMeah]

@aseemk This is really useful, and the expected behaviour would it be possible to get this merged?

[aseemk]

Sorry I missed this; thanks for the nudge.

Interesting feature. One thing I'm unsure of is how this should relate to recurse. Currently, you're doing this as the default behavior. Is that best? Or is that counterintuitive / violates least surprise?

Also, would you mind adding this to the readme in the PR? Doesn't have to be perfect; it'd just be good to see how you would communicate this feature. Thanks! (And thanks for already adding tests.)

[aseemk]

(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!

[aseemk]

Good idea btw and thanks for the PR!

[pezza3434]

Hi @aseemk,

I've changed a lot of the small things that you mentioned, I also added to the readme.

I was also unsure about the recursion functionality. I decided to omit this functionality from the recursion option for a couple of reasons:

1. It would be a major breaking change
2. The recursion options goes through the entire directory, including sub-directories and so people may want the verbosity that it exposes.
3. Looping through all the folders like that is not a 'node thing' and to resolve the folders as such would not be expected behaviour (for me anyway).

Perhaps going forwards there could be two recursion options, with and without verbosity. Verbosity would give you all the folders and contents while non verbosity would attempt to resolve each folder and treat it how we have in this pull request.

I look forward to your thoughts on the changes.

[pezza3434]

@aseemk Any updates on this? Thanks!

[aseemk]

Sorry folks, I've been unusually busy with both work and life for a sustained period of time now. I keep meaning to get to these PRs (and other projects), but don't find myself with the time + energy too often.

One thing I've done with another project is to invite one or two other people to be collaborators in the project and help maintain it. Anyone here interested in that?

I'd just ask that you be mindful of keeping this project focused, and not let it get too complex. E.g. I'm not 100% sure it's a good idea to accept all of the currently open PRs, but I haven't looked into them fully or thought deeply about it.

Apologies again for the delay, and thank you for understanding. =)

[aseemk]

Hi there,

I'm sorry I haven't been timely in responding to these issues and PRs.

If you're interested in taking over the maintainer role for this project, please let me know.

More details: #31 (comment)

Cheers,
Aseem

[yocontra]

@pezza3434 Could you rebase this? I'll try to get this merged and published ASAP. Thanks for the contribution.

[Janpot]

It makes sense that this module follows node's cjs resolution algorithm by default. I'd like to argue that recurse should follow this resolution algorithm as well.
I understand that this is a breaking change and that it would break a lot of code, including my own.
Maybe it makes sense to add a new deep option that would recurse through folders but uses require's resolution algorithm?

[yocontra]

@Janpot I agree, I think this should be the default behavior (recurse or no recurse). As for breaking code, I'll publish it as a breaking release with a note in the changelog.

[yocontra]

ping @pezza3434

[thealjey]

I need this :)