Defining relation `hasAndBelongsToMany` doesn't respect `camelCase`

[matthewmorek]

When defining tables with hasAndBelongsToMany relations and snakeCase set to true on the BaseTable, Orchid still requires the use of original table and column names in the relation definition, see below.

import { createBaseTable } from "orchid-orm";

export const BaseTable = createBaseTable({
  snakeCase: true,
  columnTypes: (t) => ({
    id: () =>
      t
        .uuid()
        .primaryKey()
        .default(t.sql`gen_random_uuid()`),
    ...t,
  }),
  exportAs: "BaseTable",
});

export class LocationTable extends BaseTable {
  readonly table = "location";
  columns = this.setColumns((t) => ({
    id: t.uuid().primaryKey(),
    name: t.varchar().unique(),
  });

  relations = {
    facilities: this.hasAndBelongsToMany(() => FacilityTable, {
      columns: ["id"],
      references: ["location_id"], // should be `locationId`
      through: {
        table: "location_facility", // should be `locationFacility`
        columns: ["facility_id"], // should be `facilityId`
        references: ["id"],
      },
    })
  }
}

export class FacilityTable extends BaseTable {
  readonly table = "facility";
  columns = this.setColumns((t) => ({
    id: t.uuid().primaryKey(),
    name: t.varchar().unique(),
  }));

  relations = {
    facilities: this.hasAndBelongsToMany(() => LocationTable, {
      columns: ["id"],
      references: ["facility_id"], // should be `facilityId`
      through: {
        table: "location_facility", // should be `locationFacility`
        columns: ["location_id"], // should be `locationId`
        references: ["id"],
      },
    }),
  };
}

export class LocationFacilityTable extends BaseTable {
  readonly table = "location_facility";
  columns = this.setColumns((t) => ({
    locationId: t.uuid().foreignKey(() => LocationTable, "id"),
    facilityId: t.uuid().foreignKey(() => FacilityTable, "id"),
    ...t.primaryKey(["facilityId", "locationId"]),
    createdAt: t.timestamp().default(t.sql`now()`),
  }));
}

It's the same when trying to execute disconnect from update() method, it won't work unless you provide original casing for the column names (see location_id).

export async function updateLocation(
  location: SubmittedLocation,
): Promise<"success" | "failure"> {
  const result = await db.location
    .findBy({ id: location.id })
    .update({
      name: location.name,
      facilities:
        location.facilities && location.facilities.length > 0
          ? {
              set: location.facilities.map((facilityId: string) => ({
                id: facilityId,
              })),
            }
          : // @ts-expect-error parameters do not respect `camelCase` naming convention
            { disconnect: { location_id: location.id } },  
    })
    .onConflict()
    .ignore();
};

Is assume that because it doesn't respect the snakeCase: true setting it's a bug, but let me know if the above is de facto the proper way to handle this type of relationship.

One more thing I discovered, is that with the following configuration (which works), if you want the TS builds to pass, you need to ignore the line { disconnect: { location_id: location.id } } in the above code, because TS expects the object key to be provided using camelCase, so locationId as opposed to location_id.

[romeerez]

I'll take a look today and will try to fix

[romeerez]

@ts-expect-error parameters do not respect camelCase naming convention

This is not related to naming, disconnect expects where conditions for the related table (facilities), not for the intermediate table.

So there will be a TS error for both locationId and location_id because facilities don't have such a column.

To delete all facilities, you can use set with an empty array:

export async function updateLocation(
  location: SubmittedLocation,
): Promise<unknown> {
  const result = await db.location
    .findBy({ id: location.id })
    .update({
      name: location.name,
      facilities: {
        set: location.facilities.map((id) => ({ id })),
      },
    })
    .onConflict()
    .ignore();
};

set will connect location with facilities by given ids, and delete previous connections if their ids aren't in the array.
And if the array is empty, it will delete all existing connections.

So I didn't apply name casting before to the joining table because it's not meant to be used anywhere in the app code, and you don't need to define that LocationFacilityTable in the code, columns are used as is.

[matthewmorek]

I tried this, but when set receives an empty array it ignores it and nothing changes. It's why I added such an elaborate conditional check with an explicit disconnect key.

[romeerez]

Then it's a bug, I'll try to fix it shortly

[romeerez]

Published a fix:

• setting to [] should be fine now, this will delete all connections (not facilities, only connections)
• using camelCase for join table columns will be translated to snake_case with respect to the snakeCase: true

[romeerez]

tbh, I don't know why this is called connect when creating but set when updating, seems like these keys are meant for the same purpose. Perhaps I should do some renaming in the future.

upd: aha, because connect won't delete any existing connections, unlike set.