Skip to content

Adds ability to pass in config object with custom fs #69

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 9 commits into
base: master
Choose a base branch
from

Conversation

kristianmandrup
Copy link

Useful for use with in-memory fs or when mocking the fs

@kristianmandrup
Copy link
Author

kristianmandrup commented Mar 25, 2019

Also allows passing options to add debugging in order to trace the internal flow.

  • which files are being ignored
  • which files are added in what order
  • if the custom File System (FS) or the default Node FS is picked up and used
  • each time it recurses into a sub folder
  • ...

All test pass. Fully compatible with previous API :)

@jergason
Copy link
Owner

jergason commented Jun 6, 2019

Hey @kristianmandrup. Thanks for this pull request. A few comments:

Unfortunately there are quite a few changes here which would make recursive-readdir break on older versions of node - let, fat arrows, async/await, etc. I don't want to drop support for those versions.

Can you help me understand the problem you're trying to solve by allowing fs to be passed in? Is this just for testing? If so, I'd recommend something like jest or rewire to mock out requires.

You mention an in-memory fs as well. What problem would using that solve?

Adding more debug logging is a good idea, but it seems pretty disconnected from the core purpose of this pull request. I'd prefer to make each pull request do one thing instead of combining multiple unrelated changes.

Ditto with adding a package-lock.json - this is unrelated to the purpose of this PR.

@kristianmandrup
Copy link
Author

Hi @jergason

I had trouble mocking fs with Jest. It's a common issue. If you know a solution, please let me know.
Perhaps I simply need to configure Jest to not do its own override/mocking? I see a lot of people having similar issues. Not obvious. Was trawling the interweb for a solution. Then decided to simply pass in a fs object to have more control. Also had trouble debugging the recursive readdir and needed more control for which files/folders to ignore and which to recurse on. These are the reasonings behind it. I might just make my own -ext version of your package for anyone with these needs, if that's alright with you.
Cheers.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants