Fix ordering of ts update overloads & fix a lot of type errors in tests

.changeset/many-snails-strive.md

@@ -0,0 +1,5 @@
+---
+"@tanstack/db": patch
+---
+
+Fix ordering of ts update overloads & fix a lot of type errors in tests

eslint.config.mjs

@@ -15,6 +15,18 @@ export default [
       "prettier/prettier": `error`,
       "stylistic/quotes": [`error`, `backtick`],
       ...prettierConfig.rules,
+      "@typescript-eslint/no-unused-vars": [
+        `error`,
+        { argsIgnorePattern: `^_`, varsIgnorePattern: `^_` },
+      ],
+      "@typescript-eslint/naming-convention": [
+        "error",
+        {
+          selector: "typeParameter",
+          format: ["PascalCase"],
+          leadingUnderscore: `allow`,
+        },
+      ],
     },
   },
 ]

examples/react/todo/src/App.tsx

@@ -130,7 +130,7 @@ const collectionsCache = new Map()
 // Function to create the appropriate todo collection based on type
 const createTodoCollection = (type: CollectionType) => {
   if (collectionsCache.has(`todo`)) {
-    return collectionsCache.get(`todo`)
+    return collectionsCache.get(`todo`) as Collection<UpdateTodo>
   } else {
     let newCollection: Collection<UpdateTodo>
     if (type === CollectionType.Electric) {
@@ -147,7 +147,7 @@ const createTodoCollection = (type: CollectionType) => {
               timestamptz: (date: string) => new Date(date),
             },
           },
-          getKey: (item) => item.id,
+          getKey: (item) => item.id!,
           schema: updateTodoSchema,
           onInsert: async ({ transaction }) => {
             const modified = transaction.mutations[0].modified
@@ -201,7 +201,7 @@ const createTodoCollection = (type: CollectionType) => {
                 : undefined,
             }))
           },
-          getKey: (item: UpdateTodo) => String(item.id),
+          getKey: (item: UpdateTodo) => item.id!,
           schema: updateTodoSchema,
           queryClient,
           onInsert: async ({ transaction }) => {
@@ -220,7 +220,7 @@ const createTodoCollection = (type: CollectionType) => {
             return await Promise.all(
               transaction.mutations.map(async (mutation) => {
                 const { original } = mutation
-                const response = await api.todos.delete(original.id)
+                await api.todos.delete(original.id)
               })
             )
           },
@@ -254,7 +254,7 @@ const createConfigCollection = (type: CollectionType) => {
               },
             },
           },
-          getKey: (item: UpdateConfig) => item.id,
+          getKey: (item: UpdateConfig) => item.id!,
           schema: updateConfigSchema,
           onInsert: async ({ transaction }) => {
             const modified = transaction.mutations[0].modified
@@ -346,7 +346,7 @@ export default function App() {
   // Always call useLiveQuery hooks
   const { data: todos } = useLiveQuery((q) =>
     q
-      .from({ todoCollection: todoCollection as Collection<UpdateTodo> })
+      .from({ todoCollection: todoCollection })
       .orderBy(`@created_at`)
       .select(`@id`, `@created_at`, `@text`, `@completed`)
   )
@@ -462,6 +462,7 @@ export default function App() {
   }
 
   const toggleTodo = (todo: UpdateTodo) => {
+    console.log(todoCollection)
     todoCollection.update(todo.id, (draft) => {
       draft.completed = !draft.completed
     })

packages/db-collections/src/electric.ts

@@ -187,7 +187,13 @@ export function electricCollectionOptions<T extends Row<unknown>>(
     : undefined
 
   // Extract standard Collection config properties
-  const { shapeOptions, onInsert, onUpdate, onDelete, ...restConfig } = config
+  const {
+    shapeOptions: _shapeOptions,
+    onInsert: _onInsert,
+    onUpdate: _onUpdate,
+    onDelete: _onDelete,
+    ...restConfig
+  } = config
 
   return {
     ...restConfig,

packages/db/src/collection.ts

@@ -903,20 +903,34 @@ export class CollectionImpl<
    * // Update with metadata
    * update("todo-1", { metadata: { reason: "user update" } }, (draft) => { draft.text = "Updated text" })
    */
+  // Overload 1: Update multiple items with a callback
   update<TItem extends object = T>(
-    key: TKey,
-    configOrCallback: ((draft: TItem) => void) | OperationConfig,
-    maybeCallback?: (draft: TItem) => void
+    key: Array<TKey | unknown>,
+    callback: (drafts: Array<TItem>) => void
+  ): TransactionType
+
+  // Overload 2: Update multiple items with config and a callback
+  update<TItem extends object = T>(
+    keys: Array<TKey | unknown>,
+    config: OperationConfig,
+    callback: (drafts: Array<TItem>) => void
+  ): TransactionType
+
+  // Overload 3: Update a single item with a callback
+  update<TItem extends object = T>(
+    id: TKey | unknown,
+    callback: (draft: TItem) => void
   ): TransactionType
 
+  // Overload 4: Update a single item with config and a callback
   update<TItem extends object = T>(
-    keys: Array<TKey>,
-    configOrCallback: ((draft: Array<TItem>) => void) | OperationConfig,
-    maybeCallback?: (draft: Array<TItem>) => void
+    id: TKey | unknown,
+    config: OperationConfig,
+    callback: (draft: TItem) => void
   ): TransactionType
 
   update<TItem extends object = T>(
-    keys: TKey | Array<TKey>,
+    keys: (TKey | unknown) | Array<TKey | unknown>,
     configOrCallback: ((draft: TItem | Array<TItem>) => void) | OperationConfig,
     maybeCallback?: (draft: TItem | Array<TItem>) => void
   ) {

packages/db/src/proxy.ts

@@ -642,7 +642,7 @@ export function createChangeProxy<
         return value
       },
 
-      set(sobj, prop, value) {
+      set(_sobj, prop, value) {
         const currentValue = changeTracker.copy_[prop as keyof T]
         debugLog(
           `set called for property ${String(prop)}, current:`,
@@ -716,7 +716,7 @@ export function createChangeProxy<
         return true
       },
 
-      defineProperty(ptarget, prop, descriptor) {
+      defineProperty(_ptarget, prop, descriptor) {
         // const result = Reflect.defineProperty(
         //   changeTracker.copy_,
         //   prop,

packages/db/src/query/schema.ts

@@ -197,7 +197,7 @@ export type SelectCallback<TContext extends Context = Context> = (
   context: TContext extends { schema: infer S } ? S : any
 ) => any
 
-export type As<TContext extends Context = Context> = string
+export type As<_TContext extends Context = Context> = string
 
 export type From<TContext extends Context = Context> = InputReference<{
   baseSchema: TContext[`baseSchema`]
@@ -219,9 +219,9 @@ export type GroupBy<TContext extends Context = Context> =
   | PropertyReference<TContext>
   | Array<PropertyReference<TContext>>
 
-export type Limit<TContext extends Context = Context> = number
+export type Limit<_TContext extends Context = Context> = number
 
-export type Offset<TContext extends Context = Context> = number
+export type Offset<_TContext extends Context = Context> = number
 
 export interface BaseQuery<TContext extends Context = Context> {
   // The select clause is an array of either plain strings or objects mapping alias names

packages/db/tests/collection-getters.test.ts

@@ -14,7 +14,6 @@ describe(`Collection getters`, () => {
       commit: () => void
     }) => void
   }
-  let mockMutationFn: { persist: () => Promise<void> }
   let config: CollectionConfig
 
   beforeEach(() => {
@@ -34,10 +33,6 @@ describe(`Collection getters`, () => {
       }),
     }
 
-    mockMutationFn = {
-      persist: vi.fn().mockResolvedValue(undefined),
-    }
-
     config = {
       id: `test-collection`,
       getKey: (val) => val.id as string,

packages/db/tests/collection-subscribe-changes.test.ts

@@ -5,9 +5,8 @@ import { createTransaction } from "../src/transactions"
 import type {
   ChangeMessage,
   ChangesPayload,
+  MutationFn,
   PendingMutation,
-  Transaction,
-  TransactionConfig,
 } from "../src/types"
 
 // Helper function to wait for changes to be processed
@@ -226,7 +225,9 @@ describe(`Collection.subscribeChanges`, () => {
       updated?: boolean
     }>({
       id: `optimistic-changes-test`,
-      getKey: (item) => item.id,
+      getKey: (item) => {
+        return item.id
+      },
       sync: {
         sync: ({ begin, write, commit }) => {
           // Listen for sync events
@@ -246,11 +247,7 @@ describe(`Collection.subscribeChanges`, () => {
       },
     })
 
-    const mutationFn = async ({
-      transaction,
-    }: {
-      transaction: Transaction
-    }) => {
+    const mutationFn: MutationFn = async ({ transaction }) => {
       emitter.emit(`sync`, transaction.mutations)
       return Promise.resolve()
     }
@@ -376,11 +373,7 @@ describe(`Collection.subscribeChanges`, () => {
       },
     })
 
-    const mutationFn = async ({
-      transaction,
-    }: {
-      transaction: Transaction
-    }) => {
+    const mutationFn: MutationFn = async ({ transaction }) => {
       emitter.emit(`sync`, transaction.mutations)
       return Promise.resolve()
     }
@@ -545,11 +538,7 @@ describe(`Collection.subscribeChanges`, () => {
         },
       },
     })
-    const mutationFn = async ({
-      transaction,
-    }: {
-      transaction: Transaction
-    }) => {
+    const mutationFn: MutationFn = async ({ transaction }) => {
       emitter.emit(`sync`, transaction.mutations)
       return Promise.resolve()
     }

packages/db/tests/collection.test-d.ts

@@ -0,0 +1,43 @@
+import { assertType, describe, expectTypeOf, it } from "vitest"
+import type { CollectionImpl } from "../src/collection"
+import type { OperationConfig } from "../src/types"
+
+describe(`Collection.update type tests`, () => {
+  type TypeTestItem = { id: string; value: number; optional?: boolean }
+
+  const updateMethod: CollectionImpl<TypeTestItem>[`update`] = (() => {}) as any // Dummy assignment for type checking
+
+  it(`should correctly type drafts for multi-item update with callback (Overload 1)`, () => {
+    updateMethod([`id1`, `id2`], (drafts) => {
+      expectTypeOf(drafts).toEqualTypeOf<Array<TypeTestItem>>()
+      // @ts-expect-error - This line should error because drafts is an array, not a single item.
+      assertType<TypeTestItem>(drafts)
+    })
+  })
+
+  it(`should correctly type drafts for multi-item update with config and callback (Overload 2)`, () => {
+    const config: OperationConfig = { metadata: { test: true } }
+    updateMethod([`id1`, `id2`], config, (drafts) => {
+      expectTypeOf(drafts).toEqualTypeOf<Array<TypeTestItem>>()
+      // @ts-expect-error - This line should error.
+      assertType<TypeTestItem>(drafts)
+    })
+  })
+
+  it(`should correctly type draft for single-item update with callback (Overload 3)`, () => {
+    updateMethod(`id1`, (draft) => {
+      expectTypeOf(draft).toEqualTypeOf<TypeTestItem>()
+      // @ts-expect-error - This line should error because draft is a single item, not an array.
+      assertType<Array<TypeTestItem>>(draft)
+    })
+  })
+
+  it(`should correctly type draft for single-item update with config and callback (Overload 4)`, () => {
+    const config: OperationConfig = { metadata: { test: true } }
+    updateMethod(`id1`, config, (draft) => {
+      expectTypeOf(draft).toEqualTypeOf<TypeTestItem>()
+      // @ts-expect-error - This line should error.
+      assertType<Array<TypeTestItem>>(draft)
+    })
+  })
+})

packages/db/tests/collection.test.ts

@@ -7,6 +7,7 @@ import type { ChangeMessage, MutationFn, PendingMutation } from "../src/types"
 
 describe(`Collection`, () => {
   it(`should throw if there's no sync config`, () => {
+    // @ts-expect-error we're testing for throwing when there's no config passed in
     expect(() => createCollection()).toThrow(`Collection requires a config`)
   })
 
@@ -200,7 +201,7 @@ describe(`Collection`, () => {
     tx.mutate(() => collection.insert(data))
 
     // @ts-expect-error possibly undefined is ok in test
-    const insertedKey = tx.mutations[0].key
+    const insertedKey = tx.mutations[0].key as string
 
     // The merged value should immediately contain the new insert
     expect(collection.state).toEqual(
@@ -410,7 +411,7 @@ describe(`Collection`, () => {
 
     // Test bulk delete
     const tx9 = createTransaction({ mutationFn })
-    tx9.mutate(() => collection.delete([keys[2], keys[3]]))
+    tx9.mutate(() => collection.delete([keys[2]!, keys[3]!]))
     // @ts-expect-error possibly undefined is ok in test
     expect(collection.state.has(keys[2])).toBe(false)
     // @ts-expect-error possibly undefined is ok in test
@@ -520,9 +521,6 @@ describe(`Collection`, () => {
       tx2.mutate(() => collection.delete(`non-existent-id`))
     ).not.toThrow()
 
-    // Should throw when trying to delete an invalid type
-    const tx3 = createTransaction({ mutationFn })
-
     // Should not throw when deleting by string key (even if key doesn't exist)
     const tx4 = createTransaction({ mutationFn })
     expect(() =>
@@ -614,7 +612,7 @@ describe(`Collection`, () => {
     )
 
     // Test delete handler
-    tx.mutate(() => collection.delete(1))
+    tx.mutate(() => collection.delete(`1`)) // Convert number to string to match expected type
 
     // Verify the handler functions were defined correctly
     // We're not testing actual invocation since that would require modifying the Collection class
@@ -625,18 +623,18 @@ describe(`Collection`, () => {
 
   it(`should execute operations outside of explicit transactions using handlers`, async () => {
     // Create handler functions that resolve after a short delay to simulate async operations
-    const onInsertMock = vi.fn().mockImplementation(async (tx) => {
+    const onInsertMock = vi.fn().mockImplementation(async () => {
       // Wait a bit to simulate an async operation
       await new Promise((resolve) => setTimeout(resolve, 10))
       return { success: true, operation: `insert` }
     })
 
-    const onUpdateMock = vi.fn().mockImplementation(async (tx) => {
+    const onUpdateMock = vi.fn().mockImplementation(async () => {
       await new Promise((resolve) => setTimeout(resolve, 10))
       return { success: true, operation: `update` }
     })
 
-    const onDeleteMock = vi.fn().mockImplementation(async (tx) => {
+    const onDeleteMock = vi.fn().mockImplementation(async () => {
       await new Promise((resolve) => setTimeout(resolve, 10))
       return { success: true, operation: `delete` }
     })
@@ -676,7 +674,7 @@ describe(`Collection`, () => {
     expect(onUpdateMock).toHaveBeenCalledTimes(1)
 
     // Test direct delete operation
-    const deleteTx = collection.delete(1)
+    const deleteTx = collection.delete(`1`) // Convert number to string to match expected type
     expect(deleteTx).toBeDefined()
     expect(onDeleteMock).toHaveBeenCalledTimes(1)
 
@@ -731,7 +729,7 @@ describe(`Collection`, () => {
 
     // Test delete without handler
     expect(() => {
-      collection.delete(1)
+      collection.delete(`1`) // Convert number to string to match expected type
     }).toThrow(
       `Collection.delete called directly (not within an explicit transaction) but no 'onDelete' handler is configured.`
     )