LegendApp / legend-state

Legend-State is a super fast and powerful state library that enables fine-grained reactivity and easy automatic persistence
https://legendapp.com/open-source/state/
MIT License
3.02k stars 87 forks source link

CRUD sync plugin: how to ensure that latest object version is synced successfully #361

Closed bananaoomarang closed 5 days ago

bananaoomarang commented 2 months ago

I am seeing inconsistent sync behavior with a persisted syncedCrud observable and wondering if I am doing something wrong or if there is a bug somewhere.

Let's say I have this observable (as part of a react-native application):

const myObservable$ = observable(syncedCrud({
    initial: {},
    fieldCreatedAt: 'timestamp',
    fieldUpdatedAt: 'timestamp',
    persist: {
      name: 'myitems',
      plugin: ObservablePersistMMKV
      retrySync: true
    },
    retry: {
      infinite: true
    },

    list: () => <promise-returning-list>,

    onSaved: ({ saved }) => ({
      timestamp: saved.timestamp,
    }),

    update: async (value) => {
      const res = await doUpdate(value.id, value)

      return {
        timestamp: res.data.timestamp,
        value: updatedValue,
      }
    },
  }))

  const state$ = syncState(myObservable)

Now let's say the user goes offline and makes a bunch of changes, then they come back online again. When state$.sync() is called I see the list function being rerun, but it doesn't seem to send any of the pending updates.

However, if I close out/reload the app, then the pending updates are sent successully on the initial load.

I do see that the pending updates appear as expected in the MMKV storage (at myitems__m), and they are successfully applied/cleared when the app loads. Is it expected that state$.sync() should trigger this process without having to reload the app?

I also notice that when retry: { infinite: true } is set, the update function is indeed retried, but if the user updates the same object multiple times whilst offline, and then returns online, the latest value is not necessarily the last update to be applied, and the server can accidentally get old data despite the pending update being removed from persistent storage.

jmeistrich commented 1 month ago

People are using sync() more than I'd expected - I use retry so it does everything automatically. So I'm looking at augmenting it. It currently just runs the get again but I'll change it to send updates too. But if you're using retry you shouldn't ever need to sync manually. If there's anything making the retry behavior not work perfectly we should fix that.

When using retry it should be cancelling previous updates and using the new data. There were some issues with that previously but they should be fixed as of alpha.30 with some more fixes in alpha.31. Is that still not working right for you? If there's issues with that could you share a repro? That is something that would absolutely need to be fixed.

bananaoomarang commented 1 month ago

Sorry this took me a minute to get back to! Double checking on the latest beta.4 I am able to reproduce a retry issue with the basic syncedCrud plugin (testing on an iOS React Native application in a simulator):

export const things$ = observable<Record<string, Thing>>(
  syncedCrud({
    initial: {},
    fieldCreatedAt: 'timestamp',
    fieldUpdatedAt: 'timestamp',
    persist: {
      name: 'things',
      retrySync: true,
    },

    retry: {
      infinite: true,
    },

    list: () => {
      return getThingsFromApi().then((res) => res.data)
    },

    onSaved: ({ saved }) => {
      return {
        timestamp: saved.timestamp,
      }
    },

    update: async (value: Thing) => {
      const id = value.id

      const res = await updateThing(
        id,
        {
            changed_bit: value.changed_bit
        }
      )

      return {
        forecast_id: id,
        timestamp: res.data.new_timestamp,
        changed_bit: value.changed_bit,
      }
    },
  })
)
  1. Go offline
  2. Make a bunch of changes to three or four Thing records (e.g. incrementing/decrementing with a button)
  3. I see a bunch of failures/retries in the console
  4. Go back online
  5. The retries start succeeding, but 'out of date' requests don't seem to be cancelled

So every 'in between' update is run and the latest update is not necessarily run last, leading to stale data being saved to the API side.

bananaoomarang commented 5 days ago

I can no longer reproduce this with beta.16, thank you @jmeistrich!