Allow using alternate sources of files

[LynnKirby]

It would be useful to abstract the filesystem API so that, for example, in-memory files can be served.

I have a proof-of-concept (or even solution?) at 6c87c36. It adds an fs property to the options which requires fs.stat and fs.createReadStream functions. Set the environment variable USE_MEMFS before running tests and the test will use memfs. memory-fs doesn't work because the fs.Stats objects it creates only has functions attached and not all of the usual properties.

Related: expressjs/serve-static#83

[dougwilson]

That PoC seems to effectively be a duplicate of #149 and would have the same concerns. The OP of #149 never followed back up on the asks. Would you be able to incorporate them into your implementation?

[dougwilson]

memory-fs doesn't work because the fs.Stats objects it creates only has functions attached and not all of the usual properties.

This sounds like my main concern with just accepting fs as an option: how do users know what works and what does not and how can we prevent accepting a fs option that will suddenly blow up later in the implementation after the initial construction?

[LynnKirby]

Documentation is easy, the hard part is deciding on the requirements.

The concern about having to increase the major version if more of fs is used is valid. I can think of two solutions:

1. Mark the feature as experimental and therefore may change between minor versions.

2. Just accept the possibility of major version changes. I looked at the library history and even in 0.0.2 only those two functions were being used. There hasn't been any change for six years (maybe, I didn't check everywhere), so is it reasonable to think this problem won't happen?

[dougwilson]

So we are intending to implement #122 to resolve a race condition which is definitively a planned change to the fs API usage currently. I think there is another open issue to use another new part of the fs API as well.

When a new major version gets cut, that becomes a larger burden because the Express.js project exposes this module as a core piece. That would that Express.js would also have to bump the major to use it and of course would mean maintaining the prior major version of this module for the LTS lifetime of the corresponding Express (which is that the prior major version of Express is supported for 1 year after a new major is released). Obviously this will happen, but it would be awesome if it doesn't require the frequency of majors to be more than one per year (which is how Node.js itself functions).

As for marking as experimental, I'm not quite sure I'm following there. Should we match how Node.js does it and make it inaccessible unless node is started with a specific command line flag?

[LynnKirby]

By experimental I meant like Node.js which enables some experimental features without a flag (e.g. currently fs.promises does this). The docs get marked with a big orange warning which is described as:

This feature is still under active development and subject to non-backwards compatible changes, or even removal, in any future version. Use of the feature is not recommended in production environments. Experimental features are not subject to the Node.js Semantic Versioning model.

There's also the third option which is probably the easiest for everyone involved. Since having an in-memory version of send and serve-static are the only the outcome that people have actually wanted from this, I could just fork the projects and make my own send-memfs and serve-memfs. Sometimes code duplication is better than parameterization.

[hinell]

@LynnKirby So is there any progress on that?

[LynnKirby]

@hinell As in prior discussion I think it would be safer to create forks of these projects instead of parameterizing them or adding a switch for memory-fs. I rarely use them so I have no interest in maintaining such forks; I just use my own privately.

[derhuerst]

I have a similar use case: I want to serve a Buffer via HTTP, with proper support for Range, ETag, Last-Modified-Since, etc.

One could argue that this is different enough to justify an entirely separate package (there's not fs.open, fs.stat and no streaming response after all), but then again send correctly implements all of these headers already and much of its complexity is about them (and not about fs).

From my perspective, an API like this would be ideal:

const buf = Buffer.from('data i want to serve', 'utf8')

send(req, '/some-data', {
  getStat: (path, cb) => cb(null, {mtime: 1234}),
  getData: (offset, bytes) => {
    const readable = new require('stream').Readable()
    readable.push(buf)
    readable.push(null)
    return readable
  }
})

I can understand though that, from the Express perspective, this is not worth maintaining.

[nicolashenry]

@derhuerst If you are interested, I developed a package named send-stream that can do it this way for express:

const express = require("express");
const stream = require("stream");

const sendStream = require("send-stream");

class CustomStorage extends sendStream.Storage {
    async open(data) {
        return {
            attachedData: data.buffer,
            mtimeMs: data.mtimeMs,
            size: data.buffer.byteLength
        };
    }
    createReadableStream(storageInfo, range, autoClose) {
        const buffer = range
            ? storageInfo.attachedData.subarray(range.start, range.end + 1)
            : storageInfo.attachedData;
        return new stream.Readable({
            autoDestroy: autoClose,
            read() {
                this.push(buffer);
                this.push(null);
            }
        });
    }
    async close() {
        // noop
    }
}

const storage = new CustomStorage();

const app = express();

const buf = Buffer.from('data i want to serve', 'utf8');
const mtimeMs = Date.now();

app.get('/some-data', async (req, res, next) => {
    try {
        await storage.send({ buffer: buf, mtimeMs: mtimeMs }, req, res);
    }
    catch (err) {
        next(err);
    }
});

app.listen(3000, () => {
    console.info('listening on http://localhost:3000');
});

[derhuerst]

@nicolashenry Oh damn, we implemented basically the same twice.

I ended up wrapping and monkey-patching (instead of forking) send: serve-buffer.

[nicolashenry]

@derhuerst My main objective with the send-stream package was to implement some features missing in send like #160 , #122 or #176 . I also added pre-compressed file serving, multi range and http2/multi framework support. It miss some documentation and it is a bit harder to use compared to send but I hope this will be useful to somebody else :)