[Explicit table names] Fix using IDs with a supertype table (#44408)

The new `GenericDatabaseReader`/`GenericDatabaseWriter` signatures added in 1.30.0 have a bug in their types: they calls like `db.get("table1", id)`, with `id: Id<"table1"> | Id<"table2">`, incorrectly type check.

Thanks for termosa on Discord for reporting the bug (https://discord.com/channels/1019350475847499849/1448756823107702805/1448781976466227250)!

### Note about making a breaking change

Changing the TypeScript types can be a breaking change in rare cases. It affects users that have custom implementations of GenericDatabaseWriter/Reader in their codebase. However, this change is simple to apply, and it is only necessary for users that have custom GenericDatabaseWriter/Reader implementations _and_ have already updated to 1.31.0.

I will update convex-test and convex-helpers after 1.31.2 is released to update the types they use.

GitOrigin-RevId: 9b9c9b6ba96eec6e959cd3eba06ef4a3e3a122d7

Author: Nicolapps

CHANGELOG.md

@@ -1,5 +1,12 @@
 # Changelog
 
+## 1.31.2 (upcoming)
+
+- Bug fix: the TypeScript types of the new `ctx.db` APIs introduced in 1.31.0
+  incorrectly allowed passing IDs with types broader than the table name
+  argument (e.g. `db.get("table1", id)` where `id` is
+  `Id<"table1"> | Id<"table2">`). This issue is fixed in 1.31.2.
+
 ## 1.31.0
 
 - `db.get`, `db.patch`, `db.replace`, and `db.delete` now accept a table name as

src/server/database.ts

@@ -21,8 +21,8 @@ interface BaseDatabaseReader<DataModel extends GenericDataModel> {
    * @returns - The {@link GenericDocument} of the document at the given {@link values.GenericId}, or `null` if it no longer exists.
    */
   get<TableName extends TableNamesInDataModel<DataModel>>(
-    table: NonUnion<TableName>,
-    id: GenericId<TableName>,
+    table: TableName,
+    id: GenericId<NonUnion<TableName>>,
   ): Promise<DocumentByName<DataModel, TableName> | null>;
 
   /**
@@ -185,8 +185,8 @@ export interface GenericDatabaseWriter<DataModel extends GenericDataModel>
    * specifies system fields like `_id`, they must match the document's existing field values.
    */
   patch<TableName extends TableNamesInDataModel<DataModel>>(
-    table: NonUnion<TableName>,
-    id: GenericId<TableName>,
+    table: TableName,
+    id: GenericId<NonUnion<TableName>>,
     value: PatchValue<DocumentByName<DataModel, TableName>>,
   ): Promise<void>;
 
@@ -215,8 +215,8 @@ export interface GenericDatabaseWriter<DataModel extends GenericDataModel>
    * and the database will fill them in.
    */
   replace<TableName extends TableNamesInDataModel<DataModel>>(
-    table: NonUnion<TableName>,
-    id: GenericId<TableName>,
+    table: TableName,
+    id: GenericId<NonUnion<TableName>>,
     value: WithOptionalSystemFields<DocumentByName<DataModel, TableName>>,
   ): Promise<void>;
 
@@ -239,8 +239,8 @@ export interface GenericDatabaseWriter<DataModel extends GenericDataModel>
    * @param id - The {@link values.GenericId} of the document to remove.
    */
   delete<TableName extends TableNamesInDataModel<DataModel>>(
-    table: NonUnion<TableName>,
-    id: GenericId<TableName>,
+    table: TableName,
+    id: GenericId<NonUnion<TableName>>,
   ): Promise<void>;
 
   /**

src/server/database_api.test.ts

@@ -224,6 +224,39 @@ describe("new DB APIs work with explicit table names", () => {
   });
 });
 
+test("new DB APIs can be used with multiple tables at once", () => {
+  type AllowedTableNames = "table1" | "table2";
+
+  const _schema = defineSchema({
+    table1: defineTable({
+      name: v.string(),
+    }),
+    table2: defineTable({
+      name: v.string(),
+    }),
+  });
+  type DataModel = DataModelFromSchemaDefinition<typeof _schema>;
+
+  async function _shouldTypecheck(
+    tableName: AllowedTableNames,
+    id: GenericId<AllowedTableNames>,
+  ) {
+    const db: GenericDatabaseWriter<DataModel> = setupWriter();
+    await db.get(tableName, id);
+    await db.patch(tableName, id, {
+      name: "test",
+    });
+    await db.insert(tableName, {
+      name: "test",
+    });
+    await db.insert(tableName, {
+      // @ts-expect-error -- field doesn’t exist on table1 or table2
+      incorrect: "field",
+    });
+    await db.delete(tableName, id);
+  }
+});
+
 describe("new DB APIs don’t type check if used with the wrong table", () => {
   test("get", async () => {
     const db = setupWriter();
@@ -269,17 +302,16 @@ describe("new DB APIs don’t type check if used with the wrong table", () => {
     await db.replace(tableName, id, {});
   });
 
-  test("can use Id<string> + a specific table name", async () => {
-    // Maybe we should disallow this??
-
+  test("can’t use Id<string> + a specific table name", async () => {
     const id = "my_id" as GenericId<string>;
 
     const db = setupWriter();
+    // @ts-expect-error
     await db.replace("testTable", id, {});
   });
 
   test("can use some union types", async () => {
-    const tableName: "a" | "b" = "a";
+    const tableName = "a" as "a" | "b";
     const id = "my_id" as GenericId<"a" | "b">;
 
     const db = setupWriter();
@@ -292,9 +324,56 @@ describe("new DB APIs don’t type check if used with the wrong table", () => {
 
     const tableName: string = "some table";
 
+    // Unfortunately, the types accept `string` as a table name
+    // when using a GenericDataModel, so let’s define a specific
+    // data model for this type test
+    const _schema = defineSchema({
+      someTable: defineTable({
+        name: v.string(),
+      }),
+      someTable2: defineTable({
+        name: v.string(),
+      }),
+    });
+    type DataModel = DataModelFromSchemaDefinition<typeof _schema>;
+
+    const db = setupWriter() as GenericDatabaseWriter<DataModel>;
+    await db.replace(
+      // @ts-expect-error -- `string` shouldn’t be a valid table name type!
+      tableName,
+      testId,
+      {},
+    );
+  });
+
+  test("can’t use a table name + (Id<'a' | 'b'>)", async () => {
+    const id = "my_id" as GenericId<"a" | "b">;
+
+    const db = setupWriter();
+
+    // @ts-expect-error
+    await db.get("a", id);
+    // @ts-expect-error
+    await db.patch("a", id, {});
+    // @ts-expect-error
+    await db.replace("a", id, {});
+    // @ts-expect-error
+    await db.delete("a", id);
+  });
+
+  test("can’t use a table name + (Id<'a'> | Id<'b'>)", async () => {
+    const id = "my_id" as GenericId<"a"> | GenericId<"b">;
+
     const db = setupWriter();
+
+    // @ts-expect-error
+    await db.get("a", id);
+    // @ts-expect-error
+    await db.patch("a", id, {});
+    // @ts-expect-error
+    await db.replace("a", id, {});
     // @ts-expect-error
-    await db.replace(tableName, testId, {});
+    await db.delete("a", id);
   });
 });