Store pending mutations as one row per mutation - #2854
Conversation
📝 WalkthroughWalkthroughChangesMutation persistence
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Reactor
participant PersistedObject
participant IndexedDBStorage
participant mutations
participant Sync
Reactor->>PersistedObject: waitForAllKeysToLoad
PersistedObject->>IndexedDBStorage: load persisted metadata and keys
IndexedDBStorage->>mutations: read mutation records
IndexedDBStorage-->>PersistedObject: return loaded records
PersistedObject-->>Reactor: return mutations
Reactor->>Sync: resend unconfirmed mutations
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (2)
client/packages/core/src/utils/PersistedObject.ts (1)
217-226: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winDistinguish a meta load failure from an empty store.
_getMeta()returnsnullwhen_initMeta()catches an error, because_meta.valuestaysnull. In that casewaitForAllKeysToLoad()resolves with an emptycurrentValueand no error. The Reactor treats that result as "no pending mutations":_onPendingMutationsLoaded()sets_pendingMutationsReady = true,_recoveredMutationIdsstays empty, and unconfirmed mutations from a previous session are not re-sent until a later load succeeds.Consider rejecting or logging when meta failed to load, so the caller can retry instead of proceeding with an empty view.
♻️ Proposed change
public async waitForAllKeysToLoad(): Promise<Record<K, T>> { const meta = await this._getMeta(); + if (!meta) { + throw this._meta.error ?? new Error('Unable to load persisted metadata'); + } - const keys = Object.keys(meta?.objects ?? {}) as K[]; + const keys = Object.keys(meta.objects ?? {}) as K[]; await Promise.all(keys.map((k) => this.waitForKeyToLoad(k))); return this.currentValue; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@client/packages/core/src/utils/PersistedObject.ts` around lines 217 - 226, Update waitForAllKeysToLoad() to distinguish a failed _getMeta() result from a legitimately empty store: when metadata loading returns null after _initMeta() catches an error, reject or otherwise surface the failure instead of returning currentValue. Preserve the existing empty-store behavior for successfully loaded metadata with no objects, and ensure callers such as _onPendingMutationsLoaded() can retry rather than marking pending mutations as ready.client/packages/core/src/Reactor.js (1)
583-602: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winCoordinate mutation loading and sending, and avoid the empty persistence write.
- Add a version-only invalidation path instead of
this.mutations.updateInPlace(() => {}). The current call enqueues a save that writesMETA_KEYand schedules GC without changing mutation data.- Ensure recovered mutations are sent once after both mutation loading and authentication complete.
_flushPendingMessagescan send a partially loaded mutation set, and_onPendingMutationsLoadedcan send the sameeventIdagain. The server does not deduplicateclient-event-idbefore callingpermissioned-tx/transact!.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@client/packages/core/src/Reactor.js` around lines 583 - 602, The _onPendingMutationsLoaded method must invalidate store-version caches without calling mutations.updateInPlace, avoiding an empty persistence write and GC scheduling. Coordinate recovered mutation dispatch with authentication and pending-message flushing so each recovered eventId is sent only once after both prerequisites are complete; update _flushPendingMessages and the recovered-mutation flow to defer or skip sends accordingly while preserving normal pending-message behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@client/packages/core/src/IndexedDBStorage.ts`:
- Around line 206-211: Guard the JSON.parse call in parsePendingMutationsBlob so
invalid legacy string values return an empty array instead of throwing. Preserve
the existing array validation and parsing behavior for valid JSON, allowing
splitPendingMutations and upgrade6To7 to complete.
- Around line 257-284: Update upgrade6To7 and upgrade5To6 to close the legacy
database connections returned by existingDb in finally blocks, ensuring v6Db and
v5db are closed on both successful and failed migrations while preserving the
current migration flow.
In `@client/packages/core/src/Reactor.js`:
- Around line 573-581: Update _onMergeMutation so _recoveredMutationIds is
populated only while the initial mutation load is in progress, using the
existing initial-load state or lifecycle marker checked by
_onPendingMutationsLoaded. Keep the merge return behavior unchanged and prevent
later _loadKey calls from adding recovered mutation IDs.
- Around line 993-1007: The comparison in _setPendingMutations must use the
pre-draft pending mutations snapshot, not prevMut fields from the Mutative
draft. Capture this._pendingMutations() before calling _updatePendingMutations,
then compare each existing event’s original tx-steps value with mut['tx-steps']
while preserving the current deletion and reassignment behavior.
In `@client/packages/core/src/utils/PersistedObject.ts`:
- Around line 217-226: Ensure PersistedObject.waitForAllKeysToLoad rejects or
reports failure when _getMeta() returns null instead of treating it as an empty
key set. In client/packages/core/src/utils/PersistedObject.ts lines 217-226,
preserve successful loading of all metadata keys; in
client/packages/core/src/Reactor.js lines 500-508, handle that failure by
retrying or leaving _pendingMutationsReady false, and do not invoke
_onPendingMutationsLoaded() after a failed load.
---
Nitpick comments:
In `@client/packages/core/src/Reactor.js`:
- Around line 583-602: The _onPendingMutationsLoaded method must invalidate
store-version caches without calling mutations.updateInPlace, avoiding an empty
persistence write and GC scheduling. Coordinate recovered mutation dispatch with
authentication and pending-message flushing so each recovered eventId is sent
only once after both prerequisites are complete; update _flushPendingMessages
and the recovered-mutation flow to defer or skip sends accordingly while
preserving normal pending-message behavior.
In `@client/packages/core/src/utils/PersistedObject.ts`:
- Around line 217-226: Update waitForAllKeysToLoad() to distinguish a failed
_getMeta() result from a legitimately empty store: when metadata loading returns
null after _initMeta() catches an error, reject or otherwise surface the failure
instead of returning currentValue. Preserve the existing empty-store behavior
for successfully loaded metadata with no objects, and ensure callers such as
_onPendingMutationsLoaded() can retry rather than marking pending mutations as
ready.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 79bb2ee5-5bee-4117-9d8f-c41d8591860c
📒 Files selected for processing (5)
client/packages/core/__tests__/src/Reactor.test.tsclient/packages/core/__tests__/src/utils/PersistedObject.test.tsclient/packages/core/src/IndexedDBStorage.tsclient/packages/core/src/Reactor.jsclient/packages/core/src/utils/PersistedObject.ts
| function parsePendingMutationsBlob(value: any): Array<[string, any]> { | ||
| // Older clients stored the pending mutations map as `[...map.entries()]`, | ||
| // and clients before version 6 JSON.stringified it first. | ||
| const entries = typeof value === 'string' ? JSON.parse(value) : value; | ||
| return Array.isArray(entries) ? entries : []; | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
Guard the legacy blob parse.
JSON.parse throws when the stored value is not valid JSON. parsePendingMutationsBlob runs inside blobReq.onsuccess, so the exception aborts the splitPendingMutations transaction. The old pendingMutations blob then stays in kv, no mutation rows are written, and upgrade6To7 only logs the abort. The user loses the pending mutations on the next boot because the reactor reads the new empty mutations store.
Return [] when the parse fails, so the migration still completes.
🛡️ Proposed fix
function parsePendingMutationsBlob(value: any): Array<[string, any]> {
// Older clients stored the pending mutations map as `[...map.entries()]`,
// and clients before version 6 JSON.stringified it first.
- const entries = typeof value === 'string' ? JSON.parse(value) : value;
- return Array.isArray(entries) ? entries : [];
+ let entries = value;
+ if (typeof value === 'string') {
+ try {
+ entries = JSON.parse(value);
+ } catch {
+ return [];
+ }
+ }
+ return Array.isArray(entries) ? entries : [];
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| function parsePendingMutationsBlob(value: any): Array<[string, any]> { | |
| // Older clients stored the pending mutations map as `[...map.entries()]`, | |
| // and clients before version 6 JSON.stringified it first. | |
| const entries = typeof value === 'string' ? JSON.parse(value) : value; | |
| return Array.isArray(entries) ? entries : []; | |
| } | |
| function parsePendingMutationsBlob(value: any): Array<[string, any]> { | |
| // Older clients stored the pending mutations map as `[...map.entries()]`, | |
| // and clients before version 6 JSON.stringified it first. | |
| let entries = value; | |
| if (typeof value === 'string') { | |
| try { | |
| entries = JSON.parse(value); | |
| } catch { | |
| return []; | |
| } | |
| } | |
| return Array.isArray(entries) ? entries : []; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@client/packages/core/src/IndexedDBStorage.ts` around lines 206 - 211, Guard
the JSON.parse call in parsePendingMutationsBlob so invalid legacy string values
return an empty array instead of throwing. Preserve the existing array
validation and parsing behavior for valid JSON, allowing splitPendingMutations
and upgrade6To7 to complete.
| async function upgrade6To7(appId: string, v7Db: IDBDatabase): Promise<void> { | ||
| const v6Db = await existingDb(`instant_${appId}_6`); | ||
| if (v6Db) { | ||
| const stores = ['kv', 'querySubs', 'syncSubs'] as const; | ||
| const entriesByStore: Array<[string, Array<[string, any]>]> = []; | ||
| for (const storeName of stores) { | ||
| entriesByStore.push([storeName, await readAllEntries(v6Db, storeName)]); | ||
| } | ||
| const tx = v7Db.transaction([...stores], 'readwrite'); | ||
| for (const [storeName, entries] of entriesByStore) { | ||
| const store = tx.objectStore(storeName); | ||
| for (const [key, value] of entries) { | ||
| store.put(value, key); | ||
| } | ||
| } | ||
| await new Promise((resolve, reject) => { | ||
| tx.oncomplete = (e) => resolve(e); | ||
| tx.onerror = (e) => reject(e); | ||
| tx.onabort = (e) => reject(e); | ||
| }); | ||
| } else { | ||
| // No version 6 db. If a version 5 db exists, bring its data over | ||
| // first; it lands in the old shape with the pending mutations blob | ||
| // in kv, which the split below moves to the mutations store. | ||
| await upgrade5To6(appId, v7Db); | ||
| } | ||
| await splitPendingMutations(v7Db); | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Inspect existingDb and upgrade5To6 for connection lifecycle handling.
fd -t f 'IndexedDBStorage.ts' | while IFS= read -r f; do
echo "== $f =="
rg -n -C 12 'function existingDb|function upgrade5To6|\.close\(\)' "$f"
doneRepository: instantdb/instant
Length of output: 2798
🏁 Script executed:
#!/bin/bash
sed -n '30,60p;110,175p;245,290p' client/packages/core/src/IndexedDBStorage.ts
printf '\nLegacy database lifecycle references:\n'
rg -n -C 3 'instant_\$\{appId\}_[56]|upgrade5To6|upgrade6To7|deleteDatabase' client/packages/core/src client/packages/core/__tests__/srcRepository: instantdb/instant
Length of output: 8812
Close legacy database connections after migration. existingDb returns open connections. Neither upgrade6To7 nor upgrade5To6 closes its legacy connection. Use finally blocks so v6Db and v5db close on both success and failure.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@client/packages/core/src/IndexedDBStorage.ts` around lines 257 - 284, Update
upgrade6To7 and upgrade5To6 to close the legacy database connections returned by
existingDb in finally blocks, ensuring v6Db and v5db are closed on both
successful and failed migrations while preserving the current migration flow.
| _onMergeMutation = (eventId, storageV, inMemoryV) => { | ||
| // A mutation in storage that no in-memory write claimed came from a | ||
| // previous session (or another tab). If the server never confirmed it, | ||
| // re-send it once the full set has loaded. | ||
| if (storageV && !inMemoryV && !storageV['tx-id']) { | ||
| this._recoveredMutationIds.add(eventId); | ||
| } | ||
| return inMemoryV || storageV; | ||
| }; |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
_recoveredMutationIds is never drained after the initial load.
_onPendingMutationsLoaded reads and resets _recoveredMutationIds exactly once. _onMergeMutation still adds to the set on every later _loadKey merge. PersistedObject.updateInPlace calls _loadKey for keys that are not loaded, and _writeToStorage calls _loadKey for the keysToLoad set, so merges continue after boot.
Two effects follow. The set grows for the lifetime of the reactor. An unconfirmed mutation adopted from another tab after boot is recorded but never re-sent.
Guard the write so the set only collects ids during the initial load.
♻️ Proposed fix
_onMergeMutation = (eventId, storageV, inMemoryV) => {
// A mutation in storage that no in-memory write claimed came from a
// previous session (or another tab). If the server never confirmed it,
// re-send it once the full set has loaded.
- if (storageV && !inMemoryV && !storageV['tx-id']) {
+ if (
+ !this._pendingMutationsReady &&
+ storageV &&
+ !inMemoryV &&
+ !storageV['tx-id']
+ ) {
this._recoveredMutationIds.add(eventId);
}
return inMemoryV || storageV;
};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| _onMergeMutation = (eventId, storageV, inMemoryV) => { | |
| // A mutation in storage that no in-memory write claimed came from a | |
| // previous session (or another tab). If the server never confirmed it, | |
| // re-send it once the full set has loaded. | |
| if (storageV && !inMemoryV && !storageV['tx-id']) { | |
| this._recoveredMutationIds.add(eventId); | |
| } | |
| return inMemoryV || storageV; | |
| }; | |
| _onMergeMutation = (eventId, storageV, inMemoryV) => { | |
| // A mutation in storage that no in-memory write claimed came from a | |
| // previous session (or another tab). If the server never confirmed it, | |
| // re-send it once the full set has loaded. | |
| if ( | |
| !this._pendingMutationsReady && | |
| storageV && | |
| !inMemoryV && | |
| !storageV['tx-id'] | |
| ) { | |
| this._recoveredMutationIds.add(eventId); | |
| } | |
| return inMemoryV || storageV; | |
| }; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@client/packages/core/src/Reactor.js` around lines 573 - 581, Update
_onMergeMutation so _recoveredMutationIds is populated only while the initial
mutation load is in progress, using the existing initial-load state or lifecycle
marker checked by _onPendingMutationsLoaded. Keep the merge return behavior
unchanged and prevent later _loadKey calls from adding recovered mutation IDs.
| // Replaces the pending mutations with a rewritten map. Only entries whose | ||
| // tx-steps actually changed get reassigned, so we only persist those rows. | ||
| _setPendingMutations(muts) { | ||
| this._updatePendingMutations((prev) => { | ||
| for (const eventId of Object.keys(prev)) { | ||
| if (!muts.has(eventId)) { | ||
| delete prev[eventId]; | ||
| } | ||
| } | ||
| for (const [eventId, mut] of muts) { | ||
| const prevMut = prev[eventId]; | ||
| if (!prevMut || prevMut['tx-steps'] !== mut['tx-steps']) { | ||
| prev[eventId] = mut; | ||
| } | ||
| } |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Confirm the mutative version and inspect draft property identity behaviour.
fd -t f 'package.json' -E node_modules | while IFS= read -r f; do
rg -n '"mutative"' "$f" && echo "-- $f"
done
rg -n -C 3 "from 'mutative'|require\('mutative'\)" --type=js --type=ts -g '!**/node_modules/**'Repository: instantdb/instant
Length of output: 279
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- Reactor imports and target methods ---'
rg -n -C 8 "_setPendingMutations|_updatePendingMutations|_pendingMutations|_rewriteMutations|refresh-ok|_flushPendingMessages|mutative" client/packages/core/src/Reactor.js
printf '%s\n' '--- mutative dependency and lock entries ---'
rg -n -C 3 '"mutative"|mutative@' client/packages/core/package.json client/www/package.json package-lock.json yarn.lock pnpm-lock.yaml 2>/dev/null || true
printf '%s\n' '--- all mutative imports ---'
rg -n -C 3 "mutative" client --glob '!**/node_modules/**'Repository: instantdb/instant
Length of output: 26786
🌐 Web query:
mutative 1.0.10 draft proxy property identity current draft array reference original documentation
💡 Result:
In Mutative, a draft is a mutable Proxy object that represents a node in your state tree [1][2]. When working with state, particularly regarding property identity and references, it is important to understand that Mutative creates independent draft proxies for each path to an object [3][4]. Regarding your specific interests: 1. Draft Proxy Property Identity: When the same object is referenced from multiple paths in the base state, Mutative treats them as independent entities [3]. Mutating an object through one path does not automatically reflect in other paths because each path creates its own separate draft proxy [3][4]. If you need to preserve or create shared references in the final state, you must explicitly assign one draft to another within your recipe (e.g., draft.a = draft.b) [3][4]. 2. Draft Array Reference: Similar to objects, arrays are drafted as proxies [1]. Historically, some array operations (like those using native methods) might have triggered unnecessary draft instantiations for items [5]. Mutative ensures that when you modify the draft, it produces a new immutable state [1]. If you want to verify the current value of a draft during the mutation process, you can use the current function [4]. Note that if a child node has been modified, current returns a new reference; if it has not been modified, it returns the original state reference [4]. 3. Documentation Context: The behavior described above is design-intentional and aligns with similar state management libraries like Immer [3]. While Mutative has introduced performance optimizations for array methods in newer versions, the core principle of independent path drafting remains central to its proxy-based architecture [1][5]. You can confirm whether an object is currently a draft using the isDraft utility [6][7]. For technical details on the internal ProxyDraft interface, which includes properties like original and copy, refer to the library's internal type definitions [8].
Citations:
- 1: https://mutative.js.org/docs/getting-started/concepts
- 2: https://github.com/unadlib/mutative/blob/723f9e92/website/docs/getting-started/concepts.md
- 3: https://github.com/unadlib/mutative/blob/main/website/docs/extra-topics/shared-references.md
- 4: https://github.com/unadlib/mutative?tab=readme-ov-file
- 5: perf(array): improve array method perf unadlib/mutative#75
- 6: https://github.com/unadlib/mutative/blob/main/docs/README.md
- 7: https://npmx.dev/package/mutative/v/%5E1.0.10
- 8: https://cdn.jsdelivr.net/npm/mutative@1.3.0/dist/interface.d.ts
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- PersistedObject.updateInPlace implementation ---'
sed -n '500,575p' client/packages/core/src/utils/PersistedObject.ts
printf '%s\n' '--- Reactor target method and rewrite implementation ---'
sed -n '986,1012p' client/packages/core/src/Reactor.js
sed -n '1320,1410p' client/packages/core/src/Reactor.js
printf '%s\n' '--- relevant tests ---'
rg -n -C 5 "_setPendingMutations|pendingMutations|tx-steps|rewriteMutations" client/packages/core --glob '*test*' --glob '*spec*' || trueRepository: instantdb/instant
Length of output: 26358
🏁 Script executed:
#!/bin/bash
set -eu
python3 - <<'PY'
import json, urllib.request
for url in [
"https://registry.npmjs.org/mutative/1.1.0",
"https://unpkg.com/mutative@1.1.0/dist/index.js",
]:
print(f"--- {url} ---")
with urllib.request.urlopen(url, timeout=15) as r:
data = r.read()
if url.endswith(".json"):
meta = json.loads(data)
print("version:", meta.get("version"))
print("main:", meta.get("main"))
print("module:", meta.get("module"))
print("dist:", meta.get("dist"))
else:
text = data.decode("utf-8", "replace")
for needle in ["function create", "current(", "isDraft", "Proxy"]:
print(needle, text.find(needle))
print(text[:1200])
PYRepository: instantdb/instant
Length of output: 2491
🌐 Web query:
site:github.com/unadlib/mutative v1.1.0 create enablePatches unchanged nested array draft proxy original reference
💡 Result:
In the context of the Mutative library (often discussed alongside Immer, particularly regarding enablePatches), when using create with enablePatches: true, the behavior regarding nested arrays, draft proxies, and original references is governed by the library's internal proxy-based draft tracking [1][2]. When you modify a nested structure within a draft: 1. Draft Proxies: Mutative creates independent draft proxies for objects and arrays as they are accessed [3][2]. Each node in the state tree is lazily proxied upon access [2]. 2. Original References: You can access the original, unmodified state of a draftable object by using the original() utility provided by the library [4][5]. This allows you to compare the current draft state against the original reference [4]. 3. Unchanged Nodes: If a part of the nested state is not modified, Mutative ensures structural sharing by maintaining references to the original, unchanged nodes in the resulting state [2]. The library only creates a new object (a shallow copy) for the specific path that was modified [2]. 4. Patches and Nested Arrays: When enablePatches is active, Mutative tracks these mutations to generate a set of JSON patches [1][2]. For nested arrays, if an array itself or any of its children are modified, the library records the specific operations (such as add, remove, or replace) [2]. If you are encountering unexpected behavior with nested array references being "unchanged" despite mutations, or if you need to maintain specific references between nested paths, Mutative treats paths as independent proxies [3]. To preserve shared references between two distinct paths in your state, you must explicitly assign the draft of one path to the other within the recipe function (e.g., draft.a = draft.b) [3]. Mutations to one path do not automatically propagate to other paths that share the same object in the base state [3]. If you are using Mutative and require detailed patch generation for complex nested arrays, ensure pathAsArray is configured appropriately (it defaults to true) [1][5].
Citations:
- 1: https://github.com/unadlib/mutative/blob/main/README.md
- 2: https://github.com/unadlib/mutative/blob/723f9e92/website/docs/getting-started/concepts.md
- 3: https://github.com/unadlib/mutative/blob/main/website/docs/extra-topics/shared-references.md
- 4: https://github.com/unadlib/mutative/blob/5e567cde57f869c79dc672917d155914be43edbb/test/immer/__tests__/base.ts
- 5: https://github.com/unadlib/mutative
🌐 Web query:
site:mutative.js.org current draft unchanged nested reference mutative create proxy
💡 Result:
The terms you mentioned are core concepts and features of Mutative [1], a JavaScript library for efficient immutable state updates [2]. In Mutative, the create function is used to produce a new immutable state from an existing one [1]. It achieves this by using a draft, which is a mutable Proxy object [1]. The concepts relate as follows: - Create Proxy: When you call create(baseState, (draft) => { ... }), Mutative generates a draft. This draft is a Proxy that tracks modifications made to the state [1]. - Current Draft / Unchanged: Mutative uses structural sharing [1]. If a nested part of the state is not modified, Mutative ensures the corresponding node in the next state is strictly equal to the node in the original base state [1]. - Mutative / Nested: When you modify a nested property on the draft (e.g., draft.a1.b2.c0 = 1), Mutative creates internal drafts for the accessed nodes only as needed [1]. This ensures that only the path being modified is copied and updated, while unchanged nested structures remain referenced from the original state [1]. Additionally, the library provides APIs like original() to retrieve the state before any modifications [3] and current() to capture a snapshot of the draft at a specific point in time without proxy wrappers [4]. These tools are essential for managing complex state transitions and debugging within the drafting process [4][3].
Citations:
- 1: https://mutative.js.org/docs/getting-started/concepts
- 2: https://mutative.js.org/
- 3: https://mutative.js.org/docs/api-reference/original/
- 4: https://mutative.js.org/docs/api-reference/current/
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- PersistedObject state and cache lifecycle ---'
sed -n '1,140p' client/packages/core/src/utils/PersistedObject.ts
sed -n '140,230p' client/packages/core/src/utils/PersistedObject.ts
printf '%s\n' '--- all mutation-store writes and cache invalidation ---'
rg -n -C 4 "currentValue|_pendingMutationsCache|updateInPlace\\(" client/packages/core/src/Reactor.js client/packages/core/src/utils/PersistedObject.tsRepository: instantdb/instant
Length of output: 35162
Compare tx-steps with the pre-draft value
mutations.updateInPlace uses Mutative 1.1.0. Inside its recipe, prevMut['tx-steps'] is a draft proxy, while mut['tx-steps'] is a plain array. The strict comparison is unequal even when the array is unchanged. This creates a patch and persists every mutation row whenever _setPendingMutations runs. Capture this._pendingMutations() before _updatePendingMutations and use that snapshot for the comparison.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@client/packages/core/src/Reactor.js` around lines 993 - 1007, The comparison
in _setPendingMutations must use the pre-draft pending mutations snapshot, not
prevMut fields from the Mutative draft. Capture this._pendingMutations() before
calling _updatePendingMutations, then compare each existing event’s original
tx-steps value with mut['tx-steps'] while preserving the current deletion and
reassignment behavior.
| // Loads every key we know about from meta and resolves once they have | ||
| // all been merged into currentValue. Used by stores that need their full | ||
| // contents at boot, like pending mutations. | ||
| public async waitForAllKeysToLoad(): Promise<Record<K, T>> { | ||
| const meta = await this._getMeta(); | ||
| const keys = Object.keys(meta?.objects ?? {}) as K[]; | ||
| await Promise.all(keys.map((k) => this.waitForKeyToLoad(k))); | ||
| return this.currentValue; | ||
| } | ||
|
|
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
A failed mutations load is indistinguishable from an empty mutations store. waitForAllKeysToLoad discards the metadata load error and resolves with an empty currentValue, and the Reactor then swallows any remaining error and proceeds. The reactor marks _pendingMutationsReady = true, opens dataForQuery without optimistic updates, and never re-sends the unconfirmed mutations that are still on disk. A later updateInPlace can also persist a meta record that omits those rows.
client/packages/core/src/utils/PersistedObject.ts#L217-L226: reject, or return a load-status flag, when_getMeta()returnsnull, instead of treating the missing metadata as zero keys.client/packages/core/src/Reactor.js#L500-L508: on a load failure, retry the load or keep_pendingMutationsReadyfalse, instead of calling_onPendingMutationsLoaded()from the.catch(...).then(...)chain.
📍 Affects 2 files
client/packages/core/src/utils/PersistedObject.ts#L217-L226(this comment)client/packages/core/src/Reactor.js#L500-L508
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@client/packages/core/src/utils/PersistedObject.ts` around lines 217 - 226,
Ensure PersistedObject.waitForAllKeysToLoad rejects or reports failure when
_getMeta() returns null instead of treating it as an empty key set. In
client/packages/core/src/utils/PersistedObject.ts lines 217-226, preserve
successful loading of all metadata keys; in client/packages/core/src/Reactor.js
lines 500-508, handle that failure by retrying or leaving _pendingMutationsReady
false, and do not invoke _onPendingMutationsLoaded() after a failed load.
|
View Vercel preview at instant-www-js-idb-per-mutation-storage-jsv.vercel.app. |
1 participant
Summary
Proposal for "option 2" from the frozen-tab work, stacked on #2853.
mutationsobject store, instead of one blob under thependingMutationskey inkv._pendingMutations()(a Map view) so the ~20 Reactor call sites read the same way; writes go through_updatePendingMutationsagainst a per-eventIdPersistedObject.Why
#2853's
commit()removes the renderer event loop from the commit path, but a process freeze also stops IPC threads, so a write is only safe once its bytes have left the renderer. Today every save rewrites the entire pending map (tens of MB under write load), which keeps that window wide. On the Android emulator with the cgroup freezer at random moments: stock blocked a sibling tab's auth boot in 4 of 6 freezes, commit() alone in 1 of 6, commit() plus bounded writes in 0 of 6.With one row per mutation, enqueue writes one small row, ack deletes one row, and no write transaction scales with the backlog. This also removes the O(backlog) write amplification: a client with 3,000 pending mutations no longer rewrites all of them every 100ms.
querySubsandsyncSubsalready use this one-key-per-entity shape;pendingMutationswas the outlier.Design notes
PersistedObjectalready writes only dirty keys, so the bounded-writes behavior falls out of making eventIds top-level keys. A newwaitForAllKeysToLoad()loads the full set at boot (from the store meta) and replaceskv.waitForKeyToLoad('pendingMutations')as the boot barrier._onMergeKvto a per-key merge plus a post-load pass that re-sends unconfirmed mutations. This also fixes a latent bug: the old guard readinMemoryV?.pendingMutations?.has(k)whereinMemoryVis already the Map, so it was always undefined and unconfirmed in-memory mutations were re-sent on every load._setPendingMutations) only reassign rows whosetx-stepsactually changed, so a rewrite persists just the affected rows.upgrade6To7) copieskv/querySubs/syncSubsverbatim from v6, then splits thependingMutationsblob into rows plus a meta record. If there is no v6 database it runs the existing v5 path first, so v5 clients migrate straight to v7.onupgradeneededinstead ofonsuccess. The old timing had a real race: sibling connections could resolve before the upgrading connection registered its promise and read pre-migration data. Spec ordering guaranteesonupgradeneededruns before any other connection's open succeeds.dataForQuery's gate used to open only once something wrote thependingMutationskey; it now opens when the mutations store finishes loading (or on first write). Fresh apps get optimistic results without waiting for a first server round trip to initialize the key.expo-sqlite,react-native-mmkv, AsyncStorage) treat the store name as a namespace string, so they need no changes; they just gain a newmutationsnamespace.Tests
All in
client/packages/core(pnpm exec vitest run --project node, 424 passing;pnpm exec tsc -p tsconfig.test.json --noEmitclean):kv/querySubscopied, old blob and its meta entry removedwaitForAllKeysToLoadunit test in PersistedObject.test.tsFollow-ups (not in this PR)
multiSetso a many-row rewrite stays bounded per transaction🤖 Generated with Claude Code