Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[FEATURE]: Better types #3673

Open
1 task done
ChoaibMouhrach opened this issue Dec 3, 2024 · 4 comments
Open
1 task done

[FEATURE]: Better types #3673

ChoaibMouhrach opened this issue Dec 3, 2024 · 4 comments
Labels
enhancement New feature or request

Comments

@ChoaibMouhrach
Copy link

Feature hasn't been suggested before.

  • I have verified this feature I'm about to request hasn't been suggested before.

Describe the enhancement you want to request

Issue Description

I'm trying to implement a Repository pattern in my application, but I'm running into issues with typing, especially when using the AnyPgTable type in Drizzle ORM.

The main problem is that AnyPgTable does not allow me to enforce certain field types for the tables. For instance, I want to ensure that a table used with a class has a field like id of type string and createdAt of type timestamp. However, since AnyPgTable cannot be extended, I have to work around it by using a type intersection like AnyPgTable & { id: AnyPgColumn, updatedAt: AnyPgColumn }.

While this seems like a solution, it causes errors in practice. For example, if I try to update a record with a new field, such as deletedAt, I get an error because the field isn't recognized by the type system. This issue also extends to using InferSelectModel when paired with InferSelectModel<TTable> & { id: string }, as it doesn't work properly due to type mismatches.

Goal

I am creating a series of classes to help organize the app and would like to leverage some predefined functions that my classes can inherit by extending a base class. Here is an example of how I want the classes to work:


Example Code: Repository Class

// REPO
import { AnyPgColumn, AnyPgTable } from "drizzle-orm/pg-core";
import { TConnection } from "../database";
import { eq, InferSelectModel } from "drizzle-orm";
import { CustomException } from "../lib/action";

type CustomAnyPgTable = AnyPgTable & {
  id: AnyPgColumn;
  deletedAt: AnyPgColumn;
};

export abstract class Repo<TTable extends CustomAnyPgTable> {
  abstract table: TTable;

  public async find(finder: { id: string }, tx: TConnection) {
    const entity = await tx
      .select()
      .from(this.table)
      .where(eq(this.table.id, finder.id));

    return entity ? new Entity(entity) : null;
  }

  public async findOrThrow(
    finder: Parameters<typeof this.find>[0],
    tx: TConnection
  ) {
    const user = await this.find(finder, tx);

    if (!user) {
      throw new CustomException("Not found");
    }

    return user;
  }

  public async create(inputs: TTable["$inferInsert"][], tx: TConnection) {
    const entities = await tx.insert(this.table).values(inputs).returning();
    return entities.map((user) => new Entity(user));
  }

  public async update(
    finder: { id: string },
    input: Partial<TTable["$inferSelect"]>,
    tx: TConnection
  ): Promise<void> {
    await tx.update(this.table).set(input).where(eq(this.table.id, finder.id));
  }

  public async softRemove(
    finder: { id: string },
    tx: TConnection
  ): Promise<void> {
    await tx
      .update(this.table)
      .set({
        deletedAt: new Date(),
      })
      .where(eq(this.table.id, finder.id));
  }
}

export class Entity<
  TTable extends CustomAnyPgTable,
  TEntity = InferSelectModel<TTable>
> {
  public data: TEntity;

  public constructor(data: TEntity) {
    this.data = data;
  }
}

Problem

In the softRemove method of the Repo class, the code below doesn't recognize that deletedAt is part of the CustomAnyPgTable type, and as a result, TypeScript throws an error.

await tx
  .update(this.table)
  .set({
    deletedAt: new Date(),
  })
  .where(eq(this.table.id, finder.id));

Even though deletedAt is defined in the CustomAnyPgTable type, TypeScript doesn't infer it correctly in this case, causing issues with the type system.

Example Usage of the Repository Class

import { usersTable } from "../database/schema";
import { Repo } from "./utils";

const table = usersTable;

// types
type TTable = typeof table;

export class UserRepo extends Repo<TTable> {
  public table = table;
}

Summary of Issues:

  1. Typing issue with AnyPgTable: Can't enforce specific fields (like id, createdAt) for tables using AnyPgTable.
  2. Type intersection workaround: While using AnyPgTable & { id: AnyPgColumn, deletedAt: AnyPgColumn } seems like a solution, it leads to type errors.
  3. deletedAt field not recognized: In the softRemove method, TypeScript doesn't recognize deletedAt as part of the table schema, causing errors during database updates.
@ChoaibMouhrach ChoaibMouhrach added the enhancement New feature or request label Dec 3, 2024
@CaptainStiggz
Copy link

CaptainStiggz commented Dec 3, 2024

After experimenting with drizzle yesterday, came here to post the exact same issue. 💯

I think it is actually possible (will post an example when I get to my computer, but it's unwieldy and hard to find).

@CaptainStiggz
Copy link

CaptainStiggz commented Dec 3, 2024

I think you can use something like:

type BaseTable = PgTableWithColumns<{
  name: string
  schema: string | undefined
  dialect: string
  // TODO: FIX ANY
  // would prefer Record<string, AnyPgColumn> - but that doesn't work
  columns: Record<string, any> & {
    id: PgColumn<{
      name: 'id'
      tableName: string
      dataType: ColumnDataType
      columnType: 'PgUUID'
      data: string
      driverParam: string
      notNull: boolean
      hasDefault: boolean
      isPrimaryKey: boolean
      isAutoincrement: boolean
      hasRuntimeDefault: boolean
      enumValues: undefined
    }>
  }
}>
export abstract class Repo<TTable extends BaseTable> {...}

This is pretty gross, but so far the only way I've found to achieve what you're asking. This should definitely be improved upon.

@ChoaibMouhrach
Copy link
Author

@CaptainStiggz it is gross, hope the team take a look at this

@stavros-tsioulis
Copy link

stavros-tsioulis commented Dec 8, 2024

Hello. I am also doing something similar, and I found this trick to work quite well:

import * as d from "drizzle-orm/pg-core"

const id = d.text().primaryKey()
export type PostgresTableWithId = ReturnType<typeof d.pgTable<string, { id: typeof id }>>

And then on the repository class file:

import type { PostgresTableWithId } from "..."

export class PostgresDrizzleRepository<Table extends PostgresTableWithId> {
  /* ... */
}

HOWEVER this has been working up until drizzle-orm@0.36.1
Today I upgraded to 0.36.4 and it started giving me type errors when trying to do this:

const insertResult = await this.client.db
  .insert(this.table)
  .values(item)

Of course, I understand that drizzle is still in 0.x so breaking changes like these are to be expected in minor versions (not to mention the obvious abuse of drizzle apis to achieve what we want).
It will be extremely well-received if drizzle gives us cleaner ways to implement repository patterns in our code.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants