Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

make sure we start synchronising & save documents even if initially loaded from storage #367

Conversation

diederich
Copy link

We found using IndexedDBStorageAdapter storage adapter, documents wouldn't start syncing in case they were found in the storage. Equally, they wouldn't get stored to the storage system.
I think there's a document event missing, which in the end adds the doc to both the storage & sync machinery.

This PR adds the missing event and a test.

I'm sure there's a better way for testing than hooking into the document event? Happy to adjust!

… storage

we found using `IndexedDBStorageAdapter` storage adapter, documents
wouldn't start syncing in case they were found in the storage.
Equally, they wouldn't get stored to the storage system.
I *think* there's a `document` event missing, which in the end
adds the doc to both the storage & sync machinery.

I added a test for this, but I'm sure there's a better way than hooking
into the `document` event?
@BrianHung
Copy link

Encountered bug of indexeddb not persisting on page refresh when upgrading automerge-repo from 1.2.1 to 2.0.0.

@pvh
Copy link
Member

pvh commented Aug 13, 2024

Hey thanks so much! That's why it's an alpha still... Sorry I've been out on holiday the last couple of weeks and left you hanging.

I probably won't take this patch as-is (I agree it should have a different test) but this absolutely helped me and @alexjg figure out what the problem is and it is greatly appreciated.

@diederich
Copy link
Author

No worries at all! And thanks for looking into this. Feel free to either push on top or just close this! 🙏

@pvh
Copy link
Member

pvh commented Aug 13, 2024

Okay, @diederich, take a look here: #374

I think this is a better test of the bug you describe but it's passing. Do you have any idea why you might be seeing different behaviour?

@diederich
Copy link
Author

Closing this, as fixed on main with b9a212e
Thank you! 🙏

@diederich diederich closed this Aug 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants