isomorphic-git / lightning-fs

A lean and fast 'fs' for the browser
MIT License
476 stars 47 forks source link

feat: add multi database backend supports #49

Closed snowyu closed 2 years ago

snowyu commented 4 years ago

see PouchDB Backend branch as Example.

import PouchDB from 'pouchdb-core';
import idb from 'pouchdb-adapter-idb';
import { installTry } from 'pouchdb-ex';
import upsert from 'pouchdb-upsertex';

PouchDB.plugin(idb)
PouchDB.plugin(installTry)
PouchDB.plugin(upsert)

import PouchBackend from '../PouchBackend'

import FS from "../index.js";
import test from './fs.js';

FS.register('pouchdb', PouchBackend)

const fs = new FS("testpouch", { wipe: true, backend: 'pouchdb', adapter: 'idb' });

const fs = new FS(databasename, {backend: 'pouchdb', adapter: 'idb'})
jcubic commented 3 years ago

Hi, this is great. I need to test how this works first. Will try to create an IndexedDB library inside web worker and interact with lightingFS that is outside of web worker (or service worker).

This may be very cool I will not need to have isomomorphic-git inside worker only indexedDB library.

I will play with this in the weekend.

jcubic commented 2 years ago

What I would add to the repo is an example directory with your pouchDB code (if you can share it) or another backend example and a usage example from this PR message.

snowyu commented 2 years ago

Sorry, I have corrected the typo of the link(PouchDB branch) above. The pouchdb could be published as a backend plugin of lightning-fs if merged.

The idb-keyval store could be abstracted in the next step.

jcubic commented 2 years ago

I still think that pouchdb code should be only an example if someone wants to implement a different backend, otherwise we will need to add more plugins and keep adding. I see the endless possibilities of plugins. The aim of this project is to add a browser filesystem. But I don't see a reason not to allow to extend the library with a different backend, but backend (as in server-side) is not the aim of this project. So I would not include the implementation of backend code as plugins. But it can be an example so if someone wants to support for instance PHP backend with JSON-RPC, he can do that (this is what I want to try to implement and see if it works with git in browser and files on the server, but I don't want to include my code in the library, it would be out of scope)

snowyu commented 2 years ago

I mean publish it to npm, not put it into lightning-fs directly.

Because I cannot use the lightning-fs on cordava, So I have to use pouchdb instead.

jcubic commented 2 years ago

I mean I don't like to publish plugins for backends into library. If you want to use pouchdb you will need to host the file in your own code. But I can include that as an example so it's more like documentation on how to use the new backend API.

The same as you have documentation on how to use fs and http clients in isomorphic-git. It also will not include different implementation only one that Is used and documentation how to write your own if needed.

jcubic commented 2 years ago

Another idea is to add your pouchdb code (but both how to initialize it and how to implement the backend) In README.

So it's more like documentation not as one of the plugins, just because there is one person that need pouchdb.

snowyu commented 2 years ago

Maybe you didn't understand my thoughts. Put pouchdb code into lightning-fs is my laziness. I do not like it. So no pr for this.

I need extract the pouchBackend code to publish it as an npm package.

jcubic commented 2 years ago

Then you need to provide some kind of documentation for the feature. Something that works and can be tested by the user. Your pauchDB looks like the ideal candidate for the example implementation (and you can make this with zero effort). If you don't want to use it as an example please look at the documentation for isomorphic-git https://isomorphic-git.org/docs/en/fs I'm afraid I can't merge your PR if there is no documentation because I don't think I will be able to create docs for this.

You've created the feature I think it's fair to expect that you write the documentation on how to use it.

At least update the README how to use it.

jcubic commented 2 years ago

Thanks for updating the docs. The tests are failing because of Sauce Labs. Will try to update dependencies and run tests again.

jcubic commented 2 years ago

I have a question why do you need to change the backend? Do you need to have multiple instances of FS with different backends or you just want to use a different backend? Because I've just noticed that the last change that was made, include changing the backend on init #65 Sorry I've didn't noticed this before.

jcubic commented 2 years ago

Please check PR #65 your feature is in conflict with the feature that was added after you forked the project. There are no code conflicts but both features are named the same.

I think that your code is much better than the one that is already in the library, (it use low-level swapping) but I can't merge the feature unless you refactor this completely and create a single Backend abstraction. But this will require some work.

snowyu commented 2 years ago

I have a question why do you need to change the backend? Do you need to have multiple instances of FS with different backends or you just want to use a different backend?

First, the database is not the exact word to describe(for your backend may not be a database).

Please check PR #65

Ok, I think you already understand. Yes, The main difference is that backend(#65) have to re-implement all the default idb-backend's codes.

I think that your code is much better than the one that is already in the library, (it use low-level swapping) but I can't merge the feature unless you refactor this completely and create a single Backend abstraction. But this will require some work.

There are two ways to refactor:

  1. use my way as new backend(drop the old backend way)
  2. Keep the old backend as it is, select another word as new backend option name
    • I have no idea about the proper word.

What do you think?

jcubic commented 2 years ago

I would delete the old backend code since it requires a lot of work from the user to implement almost the same code as the library itself. I don't see the point, if the user implements everything why he/she need to use lighting-fs.

So I would go with doing your approach and marking this as breaking change. So it will require to release version 5.0 The things TODO is:

jcubic commented 2 years ago

@snowyu will you have time to work on this and make it properly? I can come up with an API that you can follow. I'm asking because I wanted to add one feature and I think I will find some time to work on the library. And I can do multiple backends (replacement to key-val store) as well

jcubic commented 2 years ago

I'm closing this in favor of #91. The updated API looks like this:

import PouchBackend from '../PouchBackend'
const fs = new FS(databasename, {db: PouchBackend});

db needs to be an instance of the Database object.

This is a way simpler API, you don't need to register anything just pass low-level DB object to FS constructor.