tschaub / mock-fs

Configurable mock for the fs module
https://npmjs.org/package/mock-fs
Other
906 stars 86 forks source link

Is this project maintained? #383

Closed ryanblock closed 8 months ago

ryanblock commented 9 months ago

Hi there! Like many folks, we rely on mock-fs – it's done an excellent job for us!

In late October 2023 Node.js 20.x went into active LTS status, making it the version recommended by most teams / companies to use. As we've seen in #380 and #382, 20.x has introduced issues with mock-fs. Unfortunately, these issues have not been spoken to, let alone remediated.

As an open source maintainer, I totally understand the myriad challenges related to maintaining a library like mock-fs. That said, I think it would be helpful to understand whether the primary maintainer (@tschaub) intends to continue work on this project so folks can make decisions on how to move forward.

Related: if you're looking for volunteers to delegate commit privileges I'd be happy to raise my hand, as I'm sure would be a few others around these parts.

💕🙏🏼

tschaub commented 9 months ago

I'm happy to help review changes if anybody is interested in contributing fixes.

The project has benefitted from @3cp's significant contributions over time. I am not making use of it in many projects, so am not dedicating time to investigating fixes for the issues mentioned above.

ryanblock commented 8 months ago

Thanks, @tschaub! Can you provide input on how releases are cut for this project?

tschaub commented 8 months ago

@ryanblock - releases are pretty manual. Something along the lines of

npm version minor
git push --tags origin main && npm publish

Developing fixes and adding tests has been more of a heavy lift than cutting releases.

3cp commented 8 months ago

Nodejs internal change does make it harder and harder for mock-fs to keep up.

380 is probably fixable.

382 belongs to a group of issues related to jest. Since jest also does monkey patching of Nodejs internals, it doesn't play well with mock-fs.

There is also #358, #319, we probably has no way to support it because Nodejs would not expose the new fs_dir binding.

In linked issue in #319, https://github.com/nodejs/node/issues/37746#issuecomment-798809618 Nodejs also planed to cut off process.binding() from user land, that's death sentence for mock-fs.

I started avoiding mock-fs on new projects. As a result, I am spending much less time on maintaining mock-fs. One technique I am using is to avoid direct calling of fs methods, create a wrapper file for fs, every other files are using my own version of fs. Then manually mock the limited usage in unit tests, if a logic only uses readFile, I only need to mock readFile call. When testing more parts together, I use tmp folder with real files.

ryanblock commented 8 months ago

@3cp thank you for the context, super helpful. I spent a few hours this afternoon researching how mock-fs works, and found the process.binding() issue (as well as the various uses of util.inherits(), also deprecated). Some other things I found:

Re. Jest, for whatever it's worth, I personally draw bright lines around using and supporting it (and anything else that mutates globals). To me #382 and any other issues related to Jest would probably be best categorized as #wontfix.

That said, if @tschaub is not actively investing time in the project, and you are avoiding using mock-fs in new projects, maybe the writing is on the wall. It's possible that changes in Node.js have resulted in mock-fs becoming an evolutionary dead-end. Unfortunate, but that happens sometimes. Perhaps the best approach would be to deprecate the package and possibly direct folks to different solutions?

ryanblock commented 8 months ago

A followup: after considering this thread for a couple days I opted to whip up a library that (mostly) clones mock-fs's semantics while working with the actual filesystem in a tmp folder. As a nod to this project, I called it mock-tmp.

It's still early days but so far it seems to be working well thus far in the couple projects I've implemented it in this week. Unfortunately, I suspect it will lead to a small amount of business logic changes to accommodate the temp working dir, but I guess eventually that is to be expected when working with filesystem testing.

Anyway, thanks again to @tschaub and @3cp (and everyone else who's contributed) for all your work on this library, it's been so good for so long. 💕

BadIdeaException commented 1 month ago

Nodejs internal change does make it harder and harder for mock-fs to keep up. #380 is probably fixable. #382 belongs to a group of issues related to jest. Since jest also does monkey patching of Nodejs internals, it doesn't play well with mock-fs.

There is also #358, #319, we probably has no way to support it because Nodejs would not expose the new fs_dir binding.

In linked issue in #319, nodejs/node#37746 (comment) Nodejs also planed to cut off process.binding() from user land, that's death sentence for mock-fs.

I started avoiding mock-fs on new projects. As a result, I am spending much less time on maintaining mock-fs. One technique I am using is to avoid direct calling of fs methods, create a wrapper file for fs, every other files are using my own version of fs. Then manually mock the limited usage in unit tests, if a logic only uses readFile, I only need to mock readFile call. When testing more parts together, I use tmp folder with real files.

That is sad news, because I personally am finding mock-fs a lot easier to use than the main competitor, memfs. Shout out to @tschaub and @3cp here for the great work. :heart:

I do wonder if Customization hooks (formerly known as loaders) might conceptually be a way to work around the missing process.binding()?