requireDir
requireDir copied to clipboard
Added check to see if folder is a valid node module
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
@aseemk This is really useful, and the expected behaviour would it be possible to get this merged?
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.)
Good idea btw and thanks for the PR!
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:
- It would be a major breaking change
- The recursion options goes through the entire directory, including sub-directories and so people may want the verbosity that it exposes.
- 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.
@aseemk Any updates on this? Thanks!
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. =)
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: https://github.com/aseemk/requireDir/pull/31#issuecomment-255744987
Cheers, Aseem
@pezza3434 Could you rebase this? I'll try to get this merged and published ASAP. Thanks for the contribution.
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?
@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.
ping @pezza3434
I need this :)