YjsPersister errors in case startAutoLoad is run before startAutoSave

nikgraf commented 2 months ago

Describe the bug

This is the error I see:

Error: Content is not an array undefined
    at errorNew (/node_modules/tinybase/persisters/persister-yjs/index.js:36:9)
    at _callee2$ (/node_modules/tinybase/persisters/persister-yjs/index.js:447:27)
    at tryCatch (/node_modules/@babel/runtime/helpers/regeneratorRuntime.js:45:16)
    at Generator.eval (/node_modules/@babel/runtime/helpers/regeneratorRuntime.js:133:17)
    at Generator.eval [as next] (/node_modules/@babel/runtime/helpers/regeneratorRuntime.js:74:21)
    at asyncGeneratorStep (/node_modules/@babel/runtime/helpers/asyncToGenerator.js:3:17)
    at _next (/node_modules/@babel/runtime/helpers/asyncToGenerator.js:17:9)

The issue is if I run startAutoSave before startAutoLoad it wipes the Yjs object.

Your Example Website or App

https://codesandbox.io/p/sandbox/x77hnc

Steps to Reproduce the Bug or Issue

Run this code:

const doc1 = new Doc();
const store1 = createStore();
const persister1 = createYjsPersister(store1, doc1, "space", (error) => {
  console.log("YjsPersister Error:", error);
});
await persister1.startAutoLoad();
await persister1.startAutoSave();

Expected behavior

Graceful handling of the error

Screenshots or Videos

No response

Platform

Tinybase 5.3

Additional context

No response

jamesgpearce commented 2 months ago

In your example, the document is empty so indeed it will complain when you try to load structured TinyBase data from it. The error is by design.

However, if there was TinyBase content already in that document, you shouldn't see that error.

One thing you might consider is the 'initialContent' argument so if the document is empty, the store will still get instantiated if it is empty. Something like:

await persister.load([{}, {value1: 1}]); 
// loads from doc or defaults to this new content

await persister.save();
// saves to doc (either no change, or with this new content)

// autoSave and autoLoad from here on

NB: I am happy to entertain the idea of surpressing the error if alternative initial content has been provided! - that's a good clue that the author was expecting it to be empty in some conditions. LMK.

Hope that helps!

nikgraf commented 2 months ago

Yeah in our case there is no need for initial content since the sync engine will provide the initial content. Is this a use-case that's not desired or should I provide just dummy initial content?

jamesgpearce commented 2 months ago

I see - makes sense! Ok so let me do this: if you provide any initial content (even [{},{}]) the error will be suppressed. Will that work?

nikgraf commented 2 months ago

sounds good to me 👍

jamesgpearce commented 1 month ago

Fix in https://github.com/tinyplex/tinybase/commit/5de84aa16f43f4504f05054b013fd6877e17afa0 - should appear in the next release!

tinyplex / tinybase