useShape can miss shape updates between the shape is created and subscribed to leading to loading state forever

[hedblad]

When the Electric SQL server connection is established quicker than the React subscription is set up, the shape returned by useShape will be stuck in loading until the shape is modified.

It is tricky to provide a proper reliable reproduction case. We run Postgres and Electric in Docker Compose and can only reproduce this in Chrome under some quite specific circumstances, since this is a race condition and React is involved. So I will reason about the code instead.

Here are lines 163-180 of packages/react-hooks/src/react-hooks.tsx:

  const useShapeData = React.useMemo(() => {
    let latestShapeData = parseShapeData(shape)
    const getSnapshot = () => latestShapeData
    const subscribe = (onStoreChange: () => void) =>
      shapeSubscribe(shape, () => {
        latestShapeData = parseShapeData(shape)
        onStoreChange()
      })

    return () => {
      return useSyncExternalStoreWithSelector(
        subscribe,
        getSnapshot,
        getSnapshot,
        selector
      )
    }
  }, [shape, selector])

If shape loads between the initial parseShapeData and subscribe is called by React, this change will not be picked up by React and shape will be stuck in the loading state forever (or at least until the data changes).

Or to put it in different terms: things might change between setting up the shape and subscribing to it, which means we won't pick up the shape data or loading state. Since we don't have control over when react actually calls the function subscribing to the stream, it would make sense to always update the state when the subscription is created to ensure consistency.

Changing the implementation to the following fixes the issue:

const useShapeData = useMemo(() => {
    let latestShapeData = parseShapeData(shape)
    const getSnapshot = () => latestShapeData
    const subscribe = (onStoreChange: () => void) => {
      const updateShapeData = () => {
        latestShapeData = parseShapeData(shape)
        onStoreChange()
      }
      // Trigger an update immediately since we might have missed changes 
      // between setting up the ShapeStream and subscribing to it
      updateShapeData()
      return shapeSubscribe(shape, updateShapeData)
    }
    return () => {
      return useSyncExternalStoreWithSelector(
        subscribe,
        getSnapshot,
        getSnapshot,
        selector
      )
    }
  }, [shape, selector]);

A similar issue in TanStack/query is described in this issue: TanStack/query#5443 and fixed with this PR: TanStack/query#5474

We can work around this by updating the selector in a useEffect to trigger an update but that feels like a bit of a hack.

[KyleAMathews]

Huh, very interesting! Somehow we've never encountered this but the scenario you describe sounds plausible and the tanstack bug sounds exactly the same.

Could you put together a PR w/ your fix and a similar test as Tanstack added?

Thanks for the detailed bug report!

[hedblad]

Thanks for the quick response!

Sorry for not getting back sooner, I've been trying to create a reproduction case for a test without success. I'm not sure what it is in our particular case that causes React to delay the subscription here. I suspect it's due to heavy rendering that takes precedence over subscribe since subscribe is pushed to the fiber as a passive effect.

Do you have any idea on how I could trigger this behavior in a test?

[msfstef]

Very hard to add a test case with the current structure of the code - we'd have to rewire somethings and introduce more mocks, whereas currently it relies heavily on more integrated testing. I've introduced the suggested fix along with some other changes that could have potentially made it lose some updates, so I think this should be fixed.

I've managed to reproduce the issue with some ad-hoc code changes to the source to simulate the situation, and these changes fix that issue. If it persists after this change do not hesitate to reopen this or another issue, and we'll spend more time on actually wiring things up for a test reproduction.

[hedblad]

Amazing! Thanks a lot being so responsive, quick and understanding!