[Fix] Shared sync #71
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
rkistner
previously approved these changes
Feb 20, 2024
packages/powersync-sdk-common/src/client/AbstractPowerSyncDatabase.ts
Outdated
Show resolved
Hide resolved
rkistner
approved these changes
Feb 20, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
It was recently discovered in the YJS NextJS demo app that watched uploads would take long to trigger if multiple Powersync instances were created for the same DB. This was due to the sync status not being correctly shared between the (multiple) instances. The second instance would not trigger uploads since it did not know a connection to the PowerSync instance was established by the first instance.
This same issue would manifest itself in regular apps (with a single instance per tab). Sync statuses between multiple tabs would not be shared. Subsequently opened tabs would remain in
connected: falsemode.The root cause of this issue was that the
SharedSyncImplementation.worker.tsshared worker failed to run and share syncing information between tabs and instances. The worker thew an exception when trying to access thewindowobject in the global worker scope. Thewindowobject is not directly used in this worker codebase, but it is tested against in theobject-hashlibrary. The issue seems to be cause from bumping the NextJS version to14.1.0. This issue mentioned that versions after14.0.0can incorrectly compile checks onwindowobjects. This issue seems to be resolved here vercel/next.js@cfedc52. For better compatibility theobject-hashlibrary is removed since it's not actually needed. This fixes the shared worker from trying to accesswindow.Testing
Functionality was confirmed for the YJS demo app and React Native Todolist apps.
Screen.Recording.2024-02-19.at.15.56.53.mp4