Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FIDB persistence #12

Open
lewispham opened this issue Aug 9, 2016 · 14 comments
Open

FIDB persistence #12

lewispham opened this issue Aug 9, 2016 · 14 comments

Comments

@lewispham
Copy link

Is there any options for permanent storages? If not, does FIDB have any plans for supporting this feature? Something like sync data with the real IndexedDB?

@dumbmatter
Copy link
Owner

dumbmatter commented Aug 11, 2016

Currently: no

Plans: I'd like to do it at some point, but I don't have any immediate plans to do it

Sync to real IndexedDB: The only reason I can think to do this is if you use FIDB as a cache layer, but it's currently too slow to do that, which is a separate issue (basically I used arrays to store data because it's simple, but a good binary tree could make it fast enough to open up this use case).

@lewispham
Copy link
Author

@dumbmatter It might be slower with write operations, depending on whether or not we want to wait for the data-sync operations. But it would be absolutely a huge performance boost for read operations. I personally think that using FIDB as a cache layer of IndexedDB should be the primary use case. There would be no reason to use IndexedDB after that.

@daleharvey
Copy link

I was wondering if you had at all looked into compiling any of the actual IndexedDB implementations so they could be used in Node? I know thats not the aim of the project, just asking around related projects to see if anyone has tried

@dumbmatter
Copy link
Owner

I'm not aware of anyone trying that, and I'd guess that would be a pretty huge task.

@radicalapps
Copy link

Just looking at this to provide support for dodgy browser implementations.

My previous attempt also used in-memory arrays. I persisted these to LocalStorage when changes were made. It was a non-optimal thing - basically write the whole array to Storage when any changes were made, with a little SetTimer stuff as a lame attempt to optimise. It's was ok for my use-case which is mainly read, with few writes.

I've not delved into the source just yet, but it would be great to have some kind of event to hook into when data has changed, with a link to the (native fake) Store that has changed. This would enable people to roll their own persistence.

I'm working in Typescript with WebPack and had to use

const indexedDB: IDBFactory = require("fake-indexeddb");
const IDBKeyRange: IDBKeyRange = require("fake-indexeddb/lib/FDBKeyRange");

The /auto stuff didn't work for me, but the above worked very well.

I plan to use this for production code, not just for test. I don't need persistence, but it would help lots.

Great library!

Jeff.

@CMCDragonkai
Copy link

CMCDragonkai commented Oct 19, 2021

What would it take to make fakeIndexedDB into a realIndexedDB? What parts of the source code needs to be changed to make this the case. It should be possible to build on top of leveldown which will just use the C++ leveldb library.

Then one could use level-js as an isomorphic leveldb since it works in browsers, wraps IndexedDB and IndexedDB implementation supplied by this. My usecase was to put an encryption/decryption layer on top of IndexedDB and exposing this to level-js so I could have transparent encryption/decryption.

@dumbmatter
Copy link
Owner

Currently data is stored in basically the simplest/stupidest way possible, which you can see in https://github.com/dumbmatter/fakeIndexedDB/blob/0dd50f24711d73c8a213111406ed9f683c926259/src/lib/RecordStore.ts

The end result is pretty slow performance compared to what you'd get from a more intelligent approach. And of course no persistence to disk.

Possibly you could implement another backend just by replacing that one file. Ideally that would be true. But I'm not 100% sure if it's actually true, since this is mostly code I wrote a few years ago :)

Also there are some bugs that would be nice to fix. Several can be found from the W3C tests. But I think they're all pretty rare edge cases, since there have not been much complaints about them.

@CMCDragonkai
Copy link

The end result is pretty slow performance compared to what you'd get from a more intelligent approach. And of course no persistence to disk.

Is the performance bad because of specifically RecordStore.ts or because of other reasons outside in the overall design of fakeIndexedDB?

Because if it is just RecordStore.ts, it seems simple to translate those calls into leveldb calls.

@CMCDragonkai
Copy link

Also there are some bugs that would be nice to fix. Several can be found from the W3C tests. But I think they're all pretty rare edge cases, since there have not been much complaints about them.

Where are these failing W3C tests?

@dumbmatter
Copy link
Owner

Is the performance bad because of specifically RecordStore.ts or because of other reasons outside in the overall design of fakeIndexedDB?

Probably just RecordStore, I think.

Where are these failing W3C tests?

Run yarn test-w3c, look at all the tests it says "Skipping" for - that's because it fails. You can unskip them in run-all.js, which also has some explanation for what doesn't work in some of the tests.

Also, these tests are a snapshot of the W3C web platform tests from a couple years ago, it's possible there are even more failing tests they've added since then :)

@aaronhuggins
Copy link

@dumbmatter Thanks for writing this library. I'm looking at getting this ported over to Deno, updating the WPT, and adding optional persistence via sqlite WASM binding. Super helpful discussion of the RecordStore class here in this thread!

If I can port v4 over to Deno with minimal changes and that still packages for Node, would you want a PR back for those changes? Not sure if it's easily feasible to make isomorphic (mainly due to realistic-structured-clone dep), but I'll try if you're interested.

If I can do that, would you be comfortable with a method for users to choose persistence?

My idea is an exported function which sets a variable in the module that indexedDB.open will pick up on and use the optional persistence class.

@aaronhuggins
Copy link

@dumbmatter There's some genuine complexity in all the test harnessing you've written/converted with a pretty tight dependency on Node/Jest/PhantomJS.

That would be the biggest hold-up to making this completely isomorphic between Deno/Node.

Would you be comfortable with a (potentially) breaking change to some of the tests that allows them to run in Node and Deno, and which probably removes PhantomJS in favor of Qunit's headless Chrome solution?

@dumbmatter
Copy link
Owner

realistic-structured-clone is not needed in a modern environment since there is a built-in structuredClone, I think Deno has that already. The v4 branch uses structuredClone if it's available, but I can't rely on it exclusively until Node 16 is EOL, which is not until 2024. I've never used Deno so I'm not sure if there's a good way for you to take advantage of that fact and ignore whatever problems realistic-structured-clone causes. IIRC for npm there is not a good way to specify optional dependencies based on Node version.

For the tests, the Jest/PhantomJS stuff is not really necessary, it's mostly there just to ensure I don't break compatibility with test runners a lot of people use, and to serve as examples for how to use them. At this point, I could arguably get rid of PhantomJS since it's pretty old and maybe not that commonly used, but since I already have it working I'd like to keep it if possible.

Overall... I'm open to a PR to add persistence. I'm also open to a PR to make it work in Deno, although I don't really know what that entails, so it's hard for me to say much about it. I would not accept a PR that somehow makes things worse for the typical user, who is not interested in persistence or Deno. There's also some chance I look at your PR and think it's too complicated to be worth it, in which case I might recommend you fork instead.

@aaronhuggins
Copy link

I've had better luck recently with targeting Deno and transpiling for Node ESM by modifying the imports before transpiling and packaging for Node. That said, persistence for Node would add yet another persistence store, for a total of three: the original RecordStore, support for Deno's sqlite bindings, and then Node's.

I think for now, I'll maintain as a separate fork, perodically pulling in your changes. If I'm ever satisfied that the complexity is manageable (or non-existent) then I'll make a PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants