tinyplex / tinybase

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

Clarify that etags are required for the remote persister #111

Closed bndkt closed 11 months ago

bndkt commented 11 months ago

Describe the bug

As this is the first issue I'm opening, let me start by saying what an awesome project that is, I really like the idea, the implementation, and the overall quality of this! Thank you @jamesgpearce!

Now, on to the issue I ran into: I'm adding a remote persister to a store. The persister polls the data from the HTTP endpoint every few seconds, and I expected the UI that is rendered using hooks like useTable to update as well. But somehow, this isn't happening, and I can't for the life of me figure out why.

I want make sure my assumptions are correct, if they are and you don't have an idea what the problem could be, I'm happy to create a repo with a reproduction.

This is roughly how I implemented it:

I have a provider component:

function MyOwnProvider() {
const store = useCreateStore(createStore);

useCreatePersister(
    store,
    (store) => myOwnCreateRemotePersisterFunction(store, host),
    [host],
    async (persister) => {
        if (persister) {
            await persister.startAutoLoad();
        }
    }
);

<Provider store={store}>{children}</Provider>
}

And then in a child component I just do const table = useTable("myTableName");

The data is loaded and displayed correctly, so the setup seems to be fine. But if I change the data on the server and the REST endpoint is delivering the updated data on the next poll, the store doesn't get updated and then, of course, the UI isn't updated either. But I can see on the backend that the persister keeps polling and gets delivered the updated data. So it seems to be more of a problem that the store isn't updated correctly through the persister. Any idea whether this could be a bug or whether I just didn't get how it was intended to use?

Your Example Website or App

No response

Steps to Reproduce the Bug or Issue

No response

Expected behavior

No response

Screenshots or Videos

No response

Platform

Additional context

No response

jamesgpearce commented 11 months ago

Thanks! You should definitely expect this to work!

Do you have React StrictMode on? I just fixed a problem that had that symptom in yesterday's release.

A few other questions:

bndkt commented 11 months ago

Thank you @jamesgpearce, that sent me down the right path, and I found the problem.

I'm not using strict mode, but calling an explicit load via a setTimeout yielded the desired result, so it must've been the autoloading code. And then I looked at the code more closely and saw that the listener is checking for an ETag in the HTTP-response (https://github.com/tinyplex/tinybase/blob/3203b84c1ab9f912bd4527887ad6b3bec6dead5d/src/persisters/persister-remote.ts#L41-L48) and wasn't sending one. As soon as I added an ETag to the server, it started working. Probably worth mentioning that in the docs for the remote persister. I'm happy to put a notice in there, unless this was unintentional but the code looks like you want it to only work with ETags and it also makes sense I think.

jamesgpearce commented 11 months ago

Hm, that's a good point. I think I am way too implicit about that. Thanks for the debug and suggestion!

jamesgpearce commented 11 months ago

Done. Thanks!