- 
                Notifications
    
You must be signed in to change notification settings  - Fork 109
 
fix: manual transaction support in local-only collections #723
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
          🦋 Changeset detectedLatest commit: fc100e1 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  | 
    
          More templates
 @tanstack/angular-db
 @tanstack/db
 @tanstack/db-ivm
 @tanstack/electric-db-collection
 @tanstack/query-db-collection
 @tanstack/react-db
 @tanstack/rxdb-db-collection
 @tanstack/solid-db
 @tanstack/svelte-db
 @tanstack/trailbase-db-collection
 @tanstack/vue-db
 commit:   | 
    
| 
           Actually this might affect local only collection mutations in general (not just via manual transactions).  | 
    
| 
           So it turns out we solved this in a different way in local-storage.ts — which seems a bit less fragile than this:  | 
    
          
 Yeah I noticed this, but this works because local storage has a much stronger guarantee of a unique id to operate off of. Since   | 
    
| 
           All collections have stable ids though — if one isn't passed, a UUID is generated.  | 
    
| 
           Ah ok I missed that! Will take a look today  | 
    
8881363    to
    1626573      
    Compare
  
    Signed-off-by: Marc MacLeod <[email protected]>
1626573    to
    fc100e1      
    Compare
  
    | const { initialData, onInsert, onUpdate, onDelete, id, ...restConfig } = | ||
| config | ||
| 
               | 
          ||
| const collectionId = id ?? crypto.randomUUID() | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@KyleAMathews ok updated with the uuid approach. As far as I can tell, the id automatically generated by the base collection impl is not available in time for use in the collection filtering below, so I had to generate it here.
fixes #722
🎯 Changes
Fixed an issue where acceptMutations() wasn't working for manual transactions with local-only collections. The data would appear optimistically but then disappear after the transaction committed.
When filtering mutations to find ones belonging to the collection, it was comparing against
syncResult.collection, which isnullat the timeacceptMutationsis called. This is because the collection reference gets set during the sync callback, bututilsaren't assigned to the collection until after construction completes. So all mutations were being filtered out and nothing was persisted.To resolve this, I updated the mutation filtering to compare function references instead.
✅ Checklist
pnpm test:pr.🚀 Release Impact