search

tinyplex / tinybase

The reactive data store for local‑first apps.
https://tinybase.org
MIT License
3.62k stars 72 forks source link

Clarity for setPersisted (PersistedChanges, listeners) #176

Closed waynesbrain closed 1 week ago

waynesbrain commented 1 week ago

Is your feature request related to a problem? Please describe.

I noticed that the built-in createFilePersister doesn't check if there are any changes during setPersisted and it always writes the content. However, when I created my own persister I noticed that setPersisted gets called with changes === undefined sometimes. So I changed my persister to check if (!changes) return; but I am not completely sure how that might work out.

I WAS WRONG: It doesn't always work if you add this check in createFilePersister, sometimes you are left with a blank file if a row was added before startAutoSave is called...

Also, I am wondering if my custom persister should invoke any added listeners during or after setPersisted much like what would happen inside the built-in createFilePersister with the FS watcher?

MY FINDING: No. You don't have to invoke listeners during or after a setPersisted implementation.

I am unsure if external changes to persisted state completely overwrite the Store? Or if changes to that file are merged in, i.e. in the "last write wins" pipeline.

THOUGHTS: It doesn't really matter that much since we're not trying to synchronize through the file.

By adding some logging to my custom copy of the createFilePersister I also saw that my persister getPersisted function doesn't get called after listeners are invoked (when no content or changes are being passed to those listeners. e.g. watch(filePath, () => listener());). Based on this I think that either 1) TinyBase ignores persister change events while it's calling that persister's setPersisted method OR 2) there is magic (caching? timing logic?) that I don't understand. It would be useful to know what's going on there.

Describe the solution you'd like

Clarity on how persisters integrate into the overall TinyBase process. A process diagram maybe?

Describe alternatives you've considered

I have considered not having clarity and using things that I don't fully understand, but I don't prefer it!

Additional context

I tried digging into the TinyBase code to answer some of these questions, but the understanding that I seek has eluded me.

waynesbrain commented 1 week ago

One other big question I had was - if I am writing persisted state to a simple JSON storage (e.g. file.json or even just a string) should I also be using jsonStringWithUndefined and jsonParseWithUndefined like the built-in createFilePersister?

Those functions were not exported though (or I could not find them) so the answer is not obvious to me.

ANSWER: They are needed to implement a custom persister, but they are not exported at this time.

waynesbrain commented 1 week ago

UPDATE: It doesn't always work if you add if (!changes) return; to a custom createFilePersister -> setPersisted function, sometimes you are left with a blank file if a row was added before startAutoSave is called...

waynesbrain commented 1 week ago

I got most of the answers that I needed here so I'll just close this for now. The missing exports for some internals like jsonStringWithUndefined, et. al. would be better in it's own issue.