duplicated update for multiple IndexeddbPersistence and single Y.Doc

[himself65]

Upstream: https://github.com/toeverything/AFFiNE/pull/1651

In our project case, we will create multiple IndexeddbPersistence for one Y.Doc. And we realized that it will apply duplicate data into indexeddb because this line

https://github.com/yjs/y-indexeddb/blob/217fd6da1ed12b300eba647db247af4ac1257cf2/src/y-indexeddb.js#L101

[himself65]

/cc @doodlewind

[doodlewind]

As a background note, due to the strict mode or suspense mechanism in our React project, the provider may be instantiated multiple times in parallel:

// could have duplicated execution in modern react, even without strict mode disabled
useEffect(() => {
  const provider = new IndexeddbPersistence(doc)
  // ...
  return () => provider.destroy()
})

This can result in a roughly 10KB expansion of the IndexedDB storage volume each time the page is initialized.

The downstream issue: https://github.com/toeverything/AFFiNE/issues/1488

[dmonad]

If you create multiple instances of IndexeddbPersistence, then every pageload will copy the content of the document to the indexeddb database. Also all updates are stored twice. This is the expected behavior. Note that this will be cleaned up eventually. But that's just unnecessary work..

Unfortunately, I don't think there is anything I can do from my side.. I'll add a comment on your PR as well.

I think you should fix the issue on your end and ensure that only one Indexeddb provider is instantiated.

From my very limited understanding of React useEffect is called every time element is attached/detached or when the element properties change. If you have a lot of properties, then this feature can be horribly inefficient as useEffect can be called many times before all properties are initialized.

In your case, you only want to call useEffect once and you probably don't care about all the attribute changes of your element.

Then you could just do:

const ref = useRef(null)

useEffect(() => {
  const provider = new IndexeddbPersistence(doc)
  // ...
  return () => provider.destroy()
}, [ref])

This enforces that useEffect is only called when the reference to the dom changes. If that doesn't work, you could look into "memoization". Just make sure that you initialize only one IndexeddbProvider.

[himself65]

I think destroy is not a sync function. so it won't work as expected

[himself65]

https://github.com/yjs/y-indexeddb/blob/217fd6da1ed12b300eba647db247af4ac1257cf2/src/y-indexeddb.js#L110

storeState is a async function that here we didn't really stop it even we call destroy

[dmonad]

My suggestion is still that you shouldn't create several y-indexeddb instances (one after another). It doesn't matter if destroy is asynchronous or not. The code that I wrote above will make sure that useEffect is only called once. Hence only one y-indexeddb provider should be created. Please try it out.

[himself65]

thanks for responding. Our one need is to support connect and disconnect many times. For example we will have many ydoc in storage but only enable the foreground ydoc. So which requires provider has the ability to remove the side effect correctly. But I think for the y-indexeddb provider it doesn't have.

Then I decided to remove it with a new package that maintained by affine ourselves.

https://github.com/toeverything/AFFiNE/pull/1771