fix: implement proper type-safe encoding for localStorage keys

github-actions[bot] · kevin-dp · github-actions[bot] · commit 5eed119ddf12 · 2025-11-18T09:29:19.000Z
- 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 &lt;kevin-dp@users.noreply.github.com&gt;
diff --git a/.changeset/fix-numeric-id-localStorage.md b/.changeset/fix-numeric-id-localStorage.md
@@ -2,4 +2,4 @@
 "@tanstack/db": patch
 ---
 
-Fix localStorage collections failing to update/delete items with numeric IDs. Previously, operations would target the wrong item or fail entirely 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. Numeric keys are now prefixed with `__number__` in storage to ensure consistent lookups.
+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.
diff --git a/packages/db/src/local-storage.ts b/packages/db/src/local-storage.ts
@@ -152,6 +152,43 @@ function generateUuid(): string {
   return crypto.randomUUID()
 }
 
+/**
+ * Encodes a key (string or number) into a storage-safe string format.
+ * This prevents collisions between numeric and string keys by prefixing with type information.
+ *
+ * Examples:
+ *   - number 1 → "n:1"
+ *   - string "1" → "s:1"
+ *   - string "n:1" → "s:n:1"
+ *
+ * @param key - The key to encode (string or number)
+ * @returns Type-prefixed string that is safe for storage
+ */
+function encodeStorageKey(key: string | number): string {
+  if (typeof key === `number`) {
+    return `n:${key}`
+  }
+  return `s:${key}`
+}
+
+/**
+ * Decodes a storage key back to its original form.
+ * This is the inverse of encodeStorageKey.
+ *
+ * @param encodedKey - The encoded key from storage
+ * @returns The original key (string or number)
+ */
+function decodeStorageKey(encodedKey: string): string | number {
+  if (encodedKey.startsWith(`n:`)) {
+    return Number(encodedKey.slice(2))
+  }
+  if (encodedKey.startsWith(`s:`)) {
+    return encodedKey.slice(2)
+  }
+  // Fallback for legacy data without encoding
+  return encodedKey
+}
+
 /**
  * Creates an in-memory storage implementation that mimics the StorageApi interface
  * Used as a fallback when localStorage is not available (e.g., server-side rendering)
@@ -365,7 +402,7 @@ export function localStorageCollectionOptions(
       // Convert Map to object format for storage
       const objectData: Record<string, StoredItem<any>> = {}
       dataMap.forEach((storedItem, key) => {
-        objectData[String(key)] = storedItem
+        objectData[encodeStorageKey(key)] = storedItem
       })
       const serialized = parser.stringify(objectData)
       storage.setItem(config.storageKey, serialized)
@@ -415,17 +452,11 @@ export function localStorageCollectionOptions(
     // Add new items with version keys
     params.transaction.mutations.forEach((mutation) => {
       // Use the engine's pre-computed key for consistency
-      // Prefix numeric keys to prevent collision with string keys
-      // e.g., numeric 1 => "__number__1", string "1" => "1"
-      const key =
-        typeof mutation.key === `number`
-          ? `__number__${mutation.key}`
-          : String(mutation.key)
       const storedItem: StoredItem<any> = {
         versionKey: generateUuid(),
         data: mutation.modified,
       }
-      lastKnownData.set(key, storedItem)
+      lastKnownData.set(mutation.key, storedItem)
     })
 
     // Save to storage
@@ -455,17 +486,11 @@ export function localStorageCollectionOptions(
     // Update items with new version keys
     params.transaction.mutations.forEach((mutation) => {
       // Use the engine's pre-computed key for consistency
-      // Prefix numeric keys to prevent collision with string keys
-      // e.g., numeric 1 => "__number__1", string "1" => "1"
-      const key =
-        typeof mutation.key === `number`
-          ? `__number__${mutation.key}`
-          : String(mutation.key)
       const storedItem: StoredItem<any> = {
         versionKey: generateUuid(),
         data: mutation.modified,
       }
-      lastKnownData.set(key, storedItem)
+      lastKnownData.set(mutation.key, storedItem)
     })
 
     // Save to storage
@@ -490,13 +515,7 @@ export function localStorageCollectionOptions(
     // Remove items
     params.transaction.mutations.forEach((mutation) => {
       // Use the engine's pre-computed key for consistency
-      // Prefix numeric keys to prevent collision with string keys
-      // e.g., numeric 1 => "__number__1", string "1" => "1"
-      const key =
-        typeof mutation.key === `number`
-          ? `__number__${mutation.key}`
-          : String(mutation.key)
-      lastKnownData.delete(key)
+      lastKnownData.delete(mutation.key)
     })
 
     // Save to storage
@@ -562,25 +581,18 @@ export function localStorageCollectionOptions(
     // Apply each mutation
     for (const mutation of collectionMutations) {
       // Use the engine's pre-computed key to avoid key derivation issues
-      // Prefix numeric keys to prevent collision with string keys
-      // e.g., numeric 1 => "__number__1", string "1" => "1"
-      const key =
-        typeof mutation.key === `number`
-          ? `__number__${mutation.key}`
-          : String(mutation.key)
-
       switch (mutation.type) {
         case `insert`:
         case `update`: {
           const storedItem: StoredItem<Record<string, unknown>> = {
             versionKey: generateUuid(),
             data: mutation.modified,
           }
-          lastKnownData.set(key, storedItem)
+          lastKnownData.set(mutation.key, storedItem)
           break
         }
         case `delete`: {
-          lastKnownData.delete(key)
+          lastKnownData.delete(mutation.key)
           break
         }
       }
@@ -636,7 +648,7 @@ function loadFromStorage<T extends object>(
       parsed !== null &&
       !Array.isArray(parsed)
     ) {
-      Object.entries(parsed).forEach(([key, value]) => {
+      Object.entries(parsed).forEach(([encodedKey, value]) => {
         // Runtime check to ensure the value has the expected StoredItem structure
         if (
           value &&
@@ -645,9 +657,10 @@ function loadFromStorage<T extends object>(
           `data` in value
         ) {
           const storedItem = value as StoredItem<T>
-          dataMap.set(key, storedItem)
+          const decodedKey = decodeStorageKey(encodedKey)
+          dataMap.set(decodedKey, storedItem)
         } else {
-          throw new InvalidStorageDataFormatError(storageKey, key)
+          throw new InvalidStorageDataFormatError(storageKey, encodedKey)
         }
       })
     } else {
diff --git a/packages/db/tests/local-storage.test.ts b/packages/db/tests/local-storage.test.ts
@@ -1783,13 +1783,13 @@ describe(`localStorage collection`, () => {
       expect(collection.get(2)?.completed).toBe(false)
       expect(collection.get(3)?.completed).toBe(false)
 
-      // Verify in storage - numeric keys are prefixed with "__number__"
+      // Verify in storage - numeric keys are encoded with "n:" prefix
       const storedData = mockStorage.getItem(`numeric-todos`)
       expect(storedData).toBeDefined()
       const parsed = JSON.parse(storedData!)
-      expect(parsed[`__number__1`].data.completed).toBe(true)
-      expect(parsed[`__number__2`].data.completed).toBe(false)
-      expect(parsed[`__number__3`].data.completed).toBe(false)
+      expect(parsed[`n:1`].data.completed).toBe(true)
+      expect(parsed[`n:2`].data.completed).toBe(false)
+      expect(parsed[`n:3`].data.completed).toBe(false)
 
       subscription.unsubscribe()
     })
@@ -1843,13 +1843,13 @@ describe(`localStorage collection`, () => {
       expect(collection.has(2)).toBe(true)
       expect(collection.has(3)).toBe(true)
 
-      // Verify in storage - numeric keys are prefixed with "__number__"
+      // Verify in storage - numeric keys are encoded with "n:" prefix
       const storedData = mockStorage.getItem(`numeric-todos-delete`)
       expect(storedData).toBeDefined()
       const parsed = JSON.parse(storedData!)
-      expect(parsed[`__number__1`]).toBeUndefined()
-      expect(parsed[`__number__2`]).toBeDefined()
-      expect(parsed[`__number__3`]).toBeDefined()
+      expect(parsed[`n:1`]).toBeUndefined()
+      expect(parsed[`n:2`]).toBeDefined()
+      expect(parsed[`n:3`]).toBeDefined()
 
       subscription.unsubscribe()
     })
@@ -1862,17 +1862,17 @@ describe(`localStorage collection`, () => {
       }
 
       // Pre-populate storage with numeric IDs (simulating existing data)
-      // Numeric keys are stored with "__number__" prefix
+      // Numeric keys are stored with "n:" prefix
       const existingData = {
-        __number__1: {
+        "n:1": {
           versionKey: `version-1`,
           data: { id: 1, title: `First Todo`, completed: false },
         },
-        __number__2: {
+        "n:2": {
           versionKey: `version-2`,
           data: { id: 2, title: `Second Todo`, completed: false },
         },
-        __number__3: {
+        "n:3": {
           versionKey: `version-3`,
           data: { id: 3, title: `Third Todo`, completed: false },
         },
@@ -1908,13 +1908,13 @@ describe(`localStorage collection`, () => {
       expect(collection.get(2)?.completed).toBe(false)
       expect(collection.get(3)?.completed).toBe(false)
 
-      // Verify in storage - numeric keys are prefixed with "__number__"
+      // Verify in storage - numeric keys are encoded with "n:" prefix
       const storedData = mockStorage.getItem(`numeric-todos-reload`)
       expect(storedData).toBeDefined()
       const parsed = JSON.parse(storedData!)
-      expect(parsed[`__number__1`].data.completed).toBe(true)
-      expect(parsed[`__number__2`].data.completed).toBe(false)
-      expect(parsed[`__number__3`].data.completed).toBe(false)
+      expect(parsed[`n:1`].data.completed).toBe(true)
+      expect(parsed[`n:2`].data.completed).toBe(false)
+      expect(parsed[`n:3`].data.completed).toBe(false)
 
       subscription.unsubscribe()
     })
@@ -1927,17 +1927,17 @@ describe(`localStorage collection`, () => {
       }
 
       // Pre-populate storage with numeric IDs (simulating existing data)
-      // Numeric keys are stored with "__number__" prefix
+      // Numeric keys are stored with "n:" prefix
       const existingData = {
-        __number__1: {
+        "n:1": {
           versionKey: `version-1`,
           data: { id: 1, title: `First Todo`, completed: false },
         },
-        __number__2: {
+        "n:2": {
           versionKey: `version-2`,
           data: { id: 2, title: `Second Todo`, completed: false },
         },
-        __number__3: {
+        "n:3": {
           versionKey: `version-3`,
           data: { id: 3, title: `Third Todo`, completed: false },
         },
@@ -1974,13 +1974,13 @@ describe(`localStorage collection`, () => {
       expect(collection.has(2)).toBe(true)
       expect(collection.has(3)).toBe(true)
 
-      // Verify in storage - numeric keys are prefixed with "__number__"
+      // Verify in storage - numeric keys are encoded with "n:" prefix
       const storedData = mockStorage.getItem(`numeric-todos-reload-delete`)
       expect(storedData).toBeDefined()
       const parsed = JSON.parse(storedData!)
-      expect(parsed[`__number__1`]).toBeUndefined()
-      expect(parsed[`__number__2`]).toBeDefined()
-      expect(parsed[`__number__3`]).toBeDefined()
+      expect(parsed[`n:1`]).toBeUndefined()
+      expect(parsed[`n:2`]).toBeDefined()
+      expect(parsed[`n:3`]).toBeDefined()
 
       subscription.unsubscribe()
     })
@@ -2030,12 +2030,66 @@ describe(`localStorage collection`, () => {
 
       // There should be TWO entries in storage
       expect(Object.keys(parsed).length).toBe(2)
-      // Numeric ID 1 is stored with key "__number__1"
-      expect(parsed[`__number__1`]).toBeDefined()
-      expect(parsed[`__number__1`].data.title).toBe(`Numeric ID`)
-      // String ID "1" is stored with key "1"
-      expect(parsed[`1`]).toBeDefined()
-      expect(parsed[`1`].data.title).toBe(`String ID`)
+      // Numeric ID 1 is stored with key "n:1"
+      expect(parsed[`n:1`]).toBeDefined()
+      expect(parsed[`n:1`].data.title).toBe(`Numeric ID`)
+      // String ID "1" is stored with key "s:1"
+      expect(parsed[`s:1`]).toBeDefined()
+      expect(parsed[`s:1`].data.title).toBe(`String ID`)
+
+      subscription.unsubscribe()
+    })
+
+    it(`should prevent collision between numeric key and string key that matches the encoding pattern`, async () => {
+      interface MixedIdTodo {
+        id: string | number
+        title: string
+      }
+
+      const collection = createCollection(
+        localStorageCollectionOptions<MixedIdTodo>({
+          storageKey: `collision-test-todos`,
+          storage: mockStorage,
+          storageEventApi: mockStorageEventApi,
+          getKey: (todo) => todo.id,
+        })
+      )
+
+      const subscription = collection.subscribeChanges(() => {})
+
+      // Insert item with numeric ID 1
+      const tx1 = collection.insert({
+        id: 1,
+        title: `Numeric 1`,
+      })
+      await tx1.isPersisted.promise
+
+      // Insert item with string ID "n:1" (which would collide with old "__number__1" approach)
+      const tx2 = collection.insert({
+        id: `n:1`,
+        title: `String n:1`,
+      })
+      await tx2.isPersisted.promise
+
+      // Both should exist in collection
+      expect(collection.has(1)).toBe(true)
+      expect(collection.has(`n:1`)).toBe(true)
+      expect(collection.get(1)?.title).toBe(`Numeric 1`)
+      expect(collection.get(`n:1`)?.title).toBe(`String n:1`)
+
+      // Verify in storage - they should have different encoded keys
+      const storedData = mockStorage.getItem(`collision-test-todos`)
+      expect(storedData).toBeDefined()
+      const parsed = JSON.parse(storedData!)
+
+      // There should be TWO distinct entries
+      expect(Object.keys(parsed).length).toBe(2)
+      // Numeric 1 → "n:1"
+      expect(parsed[`n:1`]).toBeDefined()
+      expect(parsed[`n:1`].data.title).toBe(`Numeric 1`)
+      // String "n:1" → "s:n:1"
+      expect(parsed[`s:n:1`]).toBeDefined()
+      expect(parsed[`s:n:1`].data.title).toBe(`String n:1`)
 
       subscription.unsubscribe()
     })
