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

Persistence in the browser #124

Closed
goloroden opened this issue Jan 18, 2014 · 20 comments
Closed

Persistence in the browser #124

goloroden opened this issue Jan 18, 2014 · 20 comments

Comments

@goloroden
Copy link

In the readme you wrote that you'll

[...] implement persistence later.

I don't want to put you in a hurry, but, do you have already any plans on when later will be?

@louischatriot
Copy link
Owner

Hello,

Very good question indeed :) I don't have a lot of time for that currently but it should not be too much work. I may do it within the next few weeks but the best way would b e for you to open a PR if you need it quickly. I'll happoly review it.

Cheers,
Louis

@louischatriot
Copy link
Owner

PS: note to self, I need to reply to this tweet once it's done: https://twitter.com/rauschma/status/424725060981903360

@negue
Copy link

negue commented Jan 26, 2014

Hey,

here I have an example for how to use the localStorage as Persistence for NeDB:

https://gist.github.com/negue/6be16350db328458d379

I don*t know if this code is correct, but it seemed to work for me:

self.db.indexes._id.insert(data);

This Example isn't optimized yet, and maybe even slow for bigger datasets.

Edit: Now everything should work (updates, deletes, indexes) with the Code of #122

@marshallswain
Copy link

@negue As I was browsing through the code, I noticed that sublime linter was "upset" about the __proto__ property. Since this is the first time that I've seen that property, I did a quick search to learn about it and found this page stating that it's going away.
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Object/proto
Not sure if you already knew about it, but FYI.

@negue
Copy link

negue commented Feb 4, 2014

Yeah this line var Index = tempDB.indexes._id.__proto__.constructor; was just there since my test Persistence was not in the normal JS File, if its landing to the File inside "browser-version/browser-specific/lib" , then there wouldn't be any need for this "proto" trick^^

@marshallswain
Copy link

I see. I've been testing it out and it seems to work great. The only issue I had with it was accidentally wiping out data by trying to run some of the other functions instead of loadDatabase(), but I don't think that will be a problem once it's merged in.

@marshallswain
Copy link

@negue How did you get updates and deletes to work? I'm using code from 122 like you mentioned (just using nedb.min.js), but any time I do an update or a delete, it ends up as though it has done an insert.

@marshallswain
Copy link

@negue Nevermind. I misunderstood how it was supposed to work. I read through the server version's persistence.js file and now see that the data file grows larger with the instructions, then you run compactDataFile to compress it back down, removing the extra commands ($$delete and record rewrites for updates). So , it appears to be working perfectly. Sorry for the trouble. :)

@marshallswain
Copy link

Now I see why this trumps TaffyDB's speed. Very well engineered.

@skotchio
Copy link

Hey guys! Thanks for the work on this feature. May you tell me when do you plan to push this one into the master? I really want to realize it in my project.

@marshallswain
Copy link

@vovan22 It seems that we ought to submit a PR with a cleaned up version of @negue's code that includes tests to show it's working. That way @louischatriot could simply read the code, run the tests, and merge the request. He's obviously busy, as am I. I'll see if I can get around to making a PR this weekend to move it forward.

@skotchio
Copy link

@marshallswain thanks. I'm very excited this feature.

@skotchio
Copy link

Hey guys. Tell me how's it going around this issue? Does anybody complete this one?

@negue
Copy link

negue commented May 9, 2014

@marshallswain What kind of tests are needed for this persistence?

Maybe I'll create them and make a proper pullrequest then.

@marshallswain
Copy link

The tests would each need to

  1. Modify the database
  2. Use localStorage API to check that the appropriate changes were appended to the string.

Probably a test for each of these would be more than enough:

  • Insert
  • Update
  • Remove

I think that would be enough since everything else kinda depends on those three.

Sorry. It's too easy to get busy.

@goloroden
Copy link
Author

What needs to be done with this issue before the pull request can be merged in?

@louischatriot
Copy link
Owner

I actually coded browser persistence myself and didn't accept the PR, this one was quite tricky to get exactly right. NeDB browser now supports persistence as of v0.11 : https://github.com/louischatriot/nedb#browser-version

@fabiobon
Copy link

I Louis,
I've tried nedb#browser-version with localStorage persistence.
After any first different document property update, document are duplicated instead of been replaced. Is it normal?
for example:
db.insert({ _id:'2', planet: 'Mars' });
db.update({ _id: '2' }, { $set: { "data.satellites": 2, "data.red": true } }, {}, function () {});
db.update({ _id: '2' }, { $set: { "data.satellites": 3, } }, {}, function () {});

db.find({ planet: 'Mars' }); returns one document, but inside of localStorage value there are three documents.

Given the limits of the size of localStorage, I think it's better to replace the document rather than duplicate it. What do you think about this?

Thanx for your job!

@fabiobon
Copy link

Sorry I've not read the documentation: yourDatabase.persistence.compactDatafile

@louischatriot
Copy link
Owner

Yeah, the documents are not dupplicates but differnet versions, and only the latest is taken into account. Your point is interesting though: given the limits on localStorage size, making a lot of updates and deletes without reloading the page (when a compaction occurs) might cause problems. Autocompaction is useful in that case, I'll add a note in the readme about that

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