Add option to provide custom file system

[RandomByte]

Motivation

We would like to use serve-index on a "virtual" file system which abstracts a physical directory structure. For example, directories on different physical locations may be joined together and presented as a single directory with the content of both.

Prior art

A popular module already providing an option to supply a custom file system is webpack which also offers a memory-fs module.
A popular alternative fs implementation is for example graceful-fs.

CC: @matz3

[RandomByte]

@dougwilson would you mind to have a look? Thanks!

[dougwilson]

Hi @RandomByte sorry I didn't see this earlier; I've been having troubles receiving emails from GitHub and there are so many notifications to go through, very sorry. It's funny passing by this now, because there is a debate going on in Node.js core right now on if fs is even supposed to be considered a "library" to be passed around like an object reference like you're using here. I'm really hesitant to accept something like this if Node.js core doesn't want fs to be used in this way.

This is the discussion for context: nodejs/node#18131

There was also several discussions around accepting abstracted fs interfaces in other modules, notable the discussion at pillarjs/send#78 at which it was landed to be the most sane option is that there should be a project that provides pluggable fs abstractions which it what we'd use instead. Said project can coordinate providing an interface that would work across all drivers and we'd know that any given driver would work and we'd get around all the down sides highlighted in that discussion.

[RandomByte]

Hi @dougwilson, Thank you for your reply.

This is an interesting discussion over at Node.js core.

if fs is even supposed to be considered a "library" to be passed around like an object reference like you're using here

I'm afraid, I'm not sure what exactly you are referring to here. Is it the fact that my PR stores a reference to fs in a variable and passes that around instead of having this single, globally defined variable? Or is it that it uses the native fs interchangeably with a custom fs object (which is being passed in)?

Regarding pillarjs/send#78: I see that there needs to be a contract between a module and its consumer on what such a custom fs object offers. (And this PR is currently falling short in this regard.)

But to get this right: Would the goal of such a pluggable fs abstraction, like you two discussed, be to eliminate the need for such a contract due to it offering a fixed set of functionality that every "driver" needs to implement? In that case the contract would be shifted to this "pluggable fs abstraction" module and the custom "driver"-module.
I.e. a driver needs to implement a fixed set of functionality even if a module like serve-index does not make use of all of it as of today. On the other hand, serve-index can make use of all the functionality at any time in the future, as the "pluggable fs abstraction" module sitting in between requires that from the driver module.

Did I get this right?

---

Besides all that, I found that the invocations of fs-functions in this module did not change for the last three years. So one could say that changing them, or adding new function calls in the future could justify a major release. As per my understanding this was one of the main concerns in pillarjs/send#78.

Even though I see the issues mentioned above, maybe this module is not the right place to start solving them?

Thank you for taking your time looking into this!

[dougwilson]

Yea, pretty much. The changing of the fs was more about send vs in here, but even that has legitimate questions regarding the interface. For example, right now one does not even know what needs to be returned by the fs.stat response. Currently this module looks for two specific values of the code property and needs the .isDirectory method, and mtime needs to be there and be a Date object, and size should be there and be a number.

I'm probably missing some there, but we need to set the right expectation for what we need o have in the incoming fs object is all, which is what lead to that whole interface conversation. If there is a better way to set the expectations and what people need to actually to do form an acceptable object to provide to fs option, I'm all ears 👍 Right now the README change in the PR doesn't help at all -- it doesn't even say which version of Node.js the fs object is supposed to be the equivalent of.

[RandomByte]

Based on the discussion above, I think a generic fs-like interface will be hard to define and even harder to maintain with possible changes in Nodes fs along the way. I would actually refrain from implementing a custom-fs option for serve-index now.

The only alternative I can think of would be to have proprietary callback parameters similar to the required fs-functions but without the coupling to Nodes fs (e.g. .getFileStat, .getFileContent, etc.). Their return value would have to be defined very specifically as well. Also, this would force consumers to add glue code to bridge from their "virtual-fs" (whatever that looks like) to the serve-index callback functions.
I don't know if that would make things easier for other projects or if they would be better off with a fork of serve-index.

For the project I'm working on, we might just implement a custom index listing ourselves. So feel free to close this PR.

[RandomByte]

We implemented a custom index listing for our project. Therefore closing this.