zen-fs / core

A filesystem, anywhere
https://zen-fs.github.io/core/
MIT License
103 stars 14 forks source link

Disable/configure `FetchFS` preloading #86

Closed 1j01 closed 1 month ago

1j01 commented 2 months ago

In the ready method of FetchFS, it aggressively pre-caches every file in the filesystem, loading every file serially and failing if any request fails:

https://github.com/zen-fs/core/blob/9fac958e2b87eb6e34359f8e992fe63176941adc/src/backends/fetch.ts#L83-L88

This is impractical for large folder structures, and in my case, fails on a file that my web server (live-server) doesn't serve, apparently having a rule to ignore dotfiles.

I employed an ugly workaround, monkey-patching ready to remove pre-caching:

ZenFS.FetchFS.prototype.ready = async function () {
    if (this._isInitialized) {
        return;
    }
    await ZenFS.IndexFS.prototype.ready.call(this);
    // omitting getData() loop here
};

There is also a todo comment about this: https://github.com/zen-fs/core/blob/9fac958e2b87eb6e34359f8e992fe63176941adc/src/backends/fetch.ts#L126-L128

It would be useful to make preloading configurable, such as using globs, with **/* matching the current behavior, but I think the default should be not to preload anything, as this was surprising to me.

james-pre commented 2 months ago

The preloading is needed for sync operations. With that in mind, I think disabling the preloading when disableAsyncCache is true would provide a clean solution to this problem. Using a glob is much more complicated, and I would prefer to minimize the dependencies. What do you think?

1j01 commented 2 months ago

I suppose I shouldn't need the synchronous operations... let's see, I have one piece of code using sync variants but with await it shouldn't be much more complicated.

It's definitely easier to add preloading behavior than to take it away, so if this is just a boolean toggle, one could still preload files in other ways, such as directly through the ZenFS.fs API, or perhaps even <link rel="preload" as="fetch">.

I would still find the preloading behavior surprising, but that could be helped with docs, describing the behavior with and without disableAsyncCache.

So yeah, sounds fine to me.

james-pre commented 2 months ago

@1j01,

Sorry for taking so long to work on this, I was on vacation. Does 0.15.2 have the functionality your looking for?

1j01 commented 1 month ago

No worries. Trying out 0.16.2, I'm still seeing GET requests for what looks like every file in the filesystem... Apparently it's never setting _disableSync because it doesn't pass this condition:

    if ('_disableSync' in mount) {
        type AsyncFS = InstanceType<ReturnType<typeof Async<new () => FilesystemOf<T>>>>;
        (mount as AsyncFS)._disableSync = config.disableAsyncCache || false;
    }

It looks like _disableSync isn't actually defined on FetchFS, and it's only optionally defined on the base class FileSystem.

james-pre commented 1 month ago

@1j01,

Could you please let me know if 0.16.3 solves the issue? It removes the ... in ... check.

1j01 commented 1 month ago

Yes, it works. Thanks!

james-pre commented 1 month ago

@1j01,

Thanks, please let me know if you experience any bugs with ZenFS or would like a feature added.