Skip to content

Commit 99a3716

Browse files
KyleAMathewsclaudegithub-actions[bot]kevin-dp
authored
Fix localStorage collections for numeric IDs (#845)
* test: add regression tests for issue #397 (update/delete targeting) Adds comprehensive test coverage for issue #397 which reported that update() and delete() operations on localStorage collections were targeting the wrong items. Investigation findings: - Bug was fixed by PR #760 (Nov 5, 2025) which changed from using config.getKey() to mutation.key for consistency - All tests pass, unable to reproduce the reported issue - Likely that recent commenter is using an outdated package version Test Coverage Added: - Test for updating correct item with multiple items present - Test for deleting correct item with multiple items present - Both tests verify collection state AND localStorage persistence Also includes detailed investigation report (INVESTIGATION_397.md) documenting the analysis, timeline, and recommendations. Related: #397, #760 * fix: resolve localStorage numeric ID key type mismatch (issue #397) Fixes #397 - Update and delete operations on localStorage collections now work correctly with numeric IDs. ## Root Cause When using numeric IDs with localStorage collections, a type mismatch occurred between numeric keys (e.g., 1, 2, 3) and string keys from localStorage (e.g., "1", "2", "3"). The issue manifested when: 1. Data was loaded from localStorage (keys become strings via JSON.parse) 2. User performed update/delete with numeric ID 3. JavaScript Map lookup failed: Map.has(1) !== Map.has("1") ## The Fix Convert all mutation.key values to strings before using with lastKnownData Map, ensuring consistency with localStorage's string-only keys. Changed in packages/db/src/local-storage.ts: - Line 419 (wrappedOnInsert): const key = String(mutation.key) - Line 455 (wrappedOnUpdate): const key = String(mutation.key) - Line 486 (wrappedOnDelete): const key = String(mutation.key) - Line 554 (acceptMutations): const key = String(mutation.key) ## Test Coverage Added 6 comprehensive test cases in packages/db/tests/local-storage.test.ts: Bug #397 test suite (lines 1618-1985): - String ID tests (baseline/regression) - Numeric ID tests (direct operations) - Numeric ID tests after loading from storage (critical case) All 43 tests pass ✅ ## Impact - ✅ Fully backward compatible (string IDs unchanged) - ✅ Numeric IDs now work correctly - ✅ No breaking changes to API ## Documentation - BUG_FIX_397.md: Detailed technical explanation - INVESTIGATION_397.md: Complete investigation report Closes #397 * chore: add changeset for numeric ID localStorage fix - Remove investigation documentation files - Add changeset for patch release * test: add test for numeric/string ID collision behavior Documents that numeric ID 1 and string ID "1" will collide in localStorage due to JSON's string-only object keys. Last write wins. This is expected behavior and a fundamental localStorage limitation. * refactor: use __number__ prefix for numeric keys instead of String() Improves fix for issue #397 to prevent collision between numeric and string IDs (e.g., numeric 1 vs string "1"). Approach: - Numeric keys: prefixed with "__number__" → __number__1 - String keys: kept as-is → "1", "first" Benefits: - ✅ Fixes numeric ID bug (original issue #397) - ✅ Prevents numeric/string ID collision - ✅ Maintains backward compatibility for string IDs - ✅ All 44 tests passing Related: #397 * chore: update changeset to focus on original bug Reworded to emphasize the actual bug (numeric IDs not working) rather than collision prevention as the main issue. * test: rename test suite to remove issue number reference Changed 'Bug #397: update/delete targeting wrong item' to 'numeric and string ID handling' to be more descriptive and issue-agnostic. * fix: implement proper type-safe encoding for localStorage keys - Replace __number__ prefix with type-safe n: and s: encoding scheme - Extract encoding/decoding logic into helper functions (encodeStorageKey, decodeStorageKey) - Prevents all possible collisions between numeric and string keys - Add test case for collision prevention between numeric 1 and string 'n:1' - Update all tests to use new encoding format Co-authored-by: Kevin <[email protected]> * test: update localStorage tests for type-safe encoding Update all localStorage collection tests to match the new type-safe key encoding format that prevents collisions between numeric and string IDs. Changes: - Update test assertions to use encoded keys ("s:1" for string "1") - Update test data setup to use encoded keys in storage - Fix verifyConsistency helper to decode storage keys properly - All 17 failing tests now correctly expect the encoded format Co-authored-by: Kevin <[email protected]> * fix: update acceptMutations tests to use encoded storage keys Tests were expecting unencoded keys (e.g., 'tx-1') but the new type-safe encoding stores string keys with the 's:' prefix (e.g., 's:tx-1'). Updated 6 test cases to access parsed storage with correct encoded keys. Co-authored-by: Kevin <[email protected]> --------- Co-authored-by: Claude <[email protected]> Co-authored-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com> Co-authored-by: Kevin <[email protected]>
1 parent 6ff0f45 commit 99a3716

File tree

3 files changed

+586
-60
lines changed

3 files changed

+586
-60
lines changed
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"@tanstack/db": patch
3+
---
4+
5+
Fix localStorage collections to properly handle numeric and string IDs without collisions. Previously, operations could target the wrong item when using numeric IDs (e.g., `id: 1`, `id: 2`) after the page reloaded, due to a type mismatch between numeric keys in memory and stringified keys from localStorage. Keys are now encoded with type prefixes (`n:` for numbers, `s:` for strings) to prevent all possible collisions between different key types.

packages/db/src/local-storage.ts

Lines changed: 47 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -152,6 +152,43 @@ function generateUuid(): string {
152152
return crypto.randomUUID()
153153
}
154154

155+
/**
156+
* Encodes a key (string or number) into a storage-safe string format.
157+
* This prevents collisions between numeric and string keys by prefixing with type information.
158+
*
159+
* Examples:
160+
* - number 1 → "n:1"
161+
* - string "1" → "s:1"
162+
* - string "n:1" → "s:n:1"
163+
*
164+
* @param key - The key to encode (string or number)
165+
* @returns Type-prefixed string that is safe for storage
166+
*/
167+
function encodeStorageKey(key: string | number): string {
168+
if (typeof key === `number`) {
169+
return `n:${key}`
170+
}
171+
return `s:${key}`
172+
}
173+
174+
/**
175+
* Decodes a storage key back to its original form.
176+
* This is the inverse of encodeStorageKey.
177+
*
178+
* @param encodedKey - The encoded key from storage
179+
* @returns The original key (string or number)
180+
*/
181+
function decodeStorageKey(encodedKey: string): string | number {
182+
if (encodedKey.startsWith(`n:`)) {
183+
return Number(encodedKey.slice(2))
184+
}
185+
if (encodedKey.startsWith(`s:`)) {
186+
return encodedKey.slice(2)
187+
}
188+
// Fallback for legacy data without encoding
189+
return encodedKey
190+
}
191+
155192
/**
156193
* Creates an in-memory storage implementation that mimics the StorageApi interface
157194
* Used as a fallback when localStorage is not available (e.g., server-side rendering)
@@ -365,7 +402,7 @@ export function localStorageCollectionOptions(
365402
// Convert Map to object format for storage
366403
const objectData: Record<string, StoredItem<any>> = {}
367404
dataMap.forEach((storedItem, key) => {
368-
objectData[String(key)] = storedItem
405+
objectData[encodeStorageKey(key)] = storedItem
369406
})
370407
const serialized = parser.stringify(objectData)
371408
storage.setItem(config.storageKey, serialized)
@@ -415,12 +452,11 @@ export function localStorageCollectionOptions(
415452
// Add new items with version keys
416453
params.transaction.mutations.forEach((mutation) => {
417454
// Use the engine's pre-computed key for consistency
418-
const key = mutation.key
419455
const storedItem: StoredItem<any> = {
420456
versionKey: generateUuid(),
421457
data: mutation.modified,
422458
}
423-
lastKnownData.set(key, storedItem)
459+
lastKnownData.set(mutation.key, storedItem)
424460
})
425461

426462
// Save to storage
@@ -450,12 +486,11 @@ export function localStorageCollectionOptions(
450486
// Update items with new version keys
451487
params.transaction.mutations.forEach((mutation) => {
452488
// Use the engine's pre-computed key for consistency
453-
const key = mutation.key
454489
const storedItem: StoredItem<any> = {
455490
versionKey: generateUuid(),
456491
data: mutation.modified,
457492
}
458-
lastKnownData.set(key, storedItem)
493+
lastKnownData.set(mutation.key, storedItem)
459494
})
460495

461496
// Save to storage
@@ -480,8 +515,7 @@ export function localStorageCollectionOptions(
480515
// Remove items
481516
params.transaction.mutations.forEach((mutation) => {
482517
// Use the engine's pre-computed key for consistency
483-
const key = mutation.key
484-
lastKnownData.delete(key)
518+
lastKnownData.delete(mutation.key)
485519
})
486520

487521
// Save to storage
@@ -547,20 +581,18 @@ export function localStorageCollectionOptions(
547581
// Apply each mutation
548582
for (const mutation of collectionMutations) {
549583
// Use the engine's pre-computed key to avoid key derivation issues
550-
const key = mutation.key
551-
552584
switch (mutation.type) {
553585
case `insert`:
554586
case `update`: {
555587
const storedItem: StoredItem<Record<string, unknown>> = {
556588
versionKey: generateUuid(),
557589
data: mutation.modified,
558590
}
559-
lastKnownData.set(key, storedItem)
591+
lastKnownData.set(mutation.key, storedItem)
560592
break
561593
}
562594
case `delete`: {
563-
lastKnownData.delete(key)
595+
lastKnownData.delete(mutation.key)
564596
break
565597
}
566598
}
@@ -616,7 +648,7 @@ function loadFromStorage<T extends object>(
616648
parsed !== null &&
617649
!Array.isArray(parsed)
618650
) {
619-
Object.entries(parsed).forEach(([key, value]) => {
651+
Object.entries(parsed).forEach(([encodedKey, value]) => {
620652
// Runtime check to ensure the value has the expected StoredItem structure
621653
if (
622654
value &&
@@ -625,9 +657,10 @@ function loadFromStorage<T extends object>(
625657
`data` in value
626658
) {
627659
const storedItem = value as StoredItem<T>
628-
dataMap.set(key, storedItem)
660+
const decodedKey = decodeStorageKey(encodedKey)
661+
dataMap.set(decodedKey, storedItem)
629662
} else {
630-
throw new InvalidStorageDataFormatError(storageKey, key)
663+
throw new InvalidStorageDataFormatError(storageKey, encodedKey)
631664
}
632665
})
633666
} else {

0 commit comments

Comments
 (0)