-
Notifications
You must be signed in to change notification settings - Fork 99
fix: allow live queries to access optimistic inserts before sync completes #641
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
base: main
Are you sure you want to change the base?
Conversation
inserts before sync completes Signed-off-by: Marc MacLeod <[email protected]>
🦋 Changeset detectedLatest commit: 294de56 The changes in this PR will be included in the next version bump. This PR includes changesets to release 12 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Signed-off-by: Marc MacLeod <[email protected]>
This could work — I think it'd be unexpected to many people as you'd show the optimistic mutation right away and then the synced data would pop in right after — typically I think people would preload the collection before going to the route & allowing optimistic mutations. I think you're use case is you only create the collection when someone does a mutation and you know in this case it's empty? Could you just preload that empty creation when you enter the route? |
Yeah I think having collection loaded on screen before rendering ui that does optimistic inserts etc is def the typical use case. But the change here shouldn't really affect that case right? As far as I can tell, this change omostlynly affects folks that are optimistically inserting before sync is started - and in this case the expectation (at least my expectation hah) is that the optimistically inserted data is immediately available.
Our use case is that we create a new shape (collection) per chat thread. So the user is on the home screen:
const messagesCollection = messageCollectionForThread('thread_xyz');
const { data, isReady } = useLiveQuery(
q => q.from({ m: messagesCollection }).orderBy(({ m }) => m.createdAt, 'desc')
); We're doing all of this in order to partition the messages queries, rather than always fetching lifetime history of all messages across all threads. I thought that this was the recommended way to partition data atm, but I'm totally open to alternative approaches. |
There was another use case that came up recently where you wouldn't want to wait for sync. When you have a left join and you don't want to block the initial render on the second collection loading. Both and that and what you're doing feel like an opt in thing though as it's a bit surprising and perhaps unwanted otherwise. |
Fair enough! Should I put it behind a flag somewhere, or what do you suggest? Maybe on useLiveQuery and similar fns?
What typical situations would hit this new condition? Wouldn't the user have to a) render a page with a live query or otherwise start sync for a collection Or something along those lines? Just trying to get a sense of how likely it is for folks to run into this use case when they are not intentionally trying to optimistically insert before sync. |
Fix optimistic inserts not showing in live queries before sync completes.
Related Discord discussion: https://discord.com/channels/933657521581858818/1374668288549847071/1423800713221505145
The Problem
We ran into an issue where if you:
The live query would return empty results until Electric finished syncing, even though the optimistic data was sitting right there in the collection.
This was breaking our use case where we create message collections on-demand (per-thread partitioning) and insert the first message before Electric has synced anything.
The Fix
The issue was in allCollectionsReadyOrInitialCommit() - it was refusing to run the graph on collections that were still idle or loading, even if they had data.
Added a simple check: if collection.size > 0, let the graph run. This allows processing optimistic data while waiting for sync to complete.
Tested in our app and confirmed the messages now show up instantly instead of waiting for Electric to sync.
Disclaimer: I am NOT an expert on this codebase.. unsure if there are downstream ramifications to this tweak, but all of the tests pass...
Side note: the PR template tells the user to verify they have followed the contributing guide @ https://github.com/TanStack/db/blob/main/CONTRIBUTING.md - which no longer exists.