Help with types in Repository Pattern

Hello, I'm trying to set up a crud repository where it can be extended but I'm not sure how to get the typings correct. Here is what I have and I'm stuck. The issue that I have is I'm not sure how to actually get the primaryKey in a type safe way.
export interface BaseRepository<T, ID> {
findAll(): Promise<T[]>;

findAll(criteria: QueryCriteria): Promise<T[]>;

findOne(id: ID): Promise<T | null>;

update(id: ID, data: T): Promise<T>;

create(data: T): Promise<T>;

delete(id: ID): Promise<void>;
}


export abstract class CrudRepository<
M extends MySqlTable,
ID extends keyof S,
S extends InferModel<M>,
I extends InferModel<M, "insert">> implements BaseRepository<S, S[ID]> {


protected constructor(protected readonly db: DbConnection,
protected readonly schema: M,
protected readonly primaryKey: ID) {
}


async create(data: S): Promise<S> {
await this.db.insert(this.schema).values([{ ...data }]);

const result = await this.db.select().from(this.schema).where(eq(this.schema[this.primaryKey], "LAST_INSERT_ID()"));
return result[0];
}

async delete(id: S[ID]): Promise<void> {
const result = await this.db.delete(this.schema).where(eq(this.schema[this.primaryKey], id));
}

async findAll(): Promise<S[]> {
return this.db.select().from(this.schema);
}

findOne(id: S[ID]): Promise<S | null> {
return this.db.select().from(this.schema).where(eq(this.schema[this.primaryKey], id));
}

async update(id: S[ID], data: I): Promise<S | null> {
const entity = this.findOne(id);
if (!entity) {
throw new Error("Entity not found");
}
this.db.update(this.schema).set(data).where(eq(this.schema[this.primaryKey], id));
return this.findOne(id);
}

}
export interface BaseRepository<T, ID> {
findAll(): Promise<T[]>;

findAll(criteria: QueryCriteria): Promise<T[]>;

findOne(id: ID): Promise<T | null>;

update(id: ID, data: T): Promise<T>;

create(data: T): Promise<T>;

delete(id: ID): Promise<void>;
}


export abstract class CrudRepository<
M extends MySqlTable,
ID extends keyof S,
S extends InferModel<M>,
I extends InferModel<M, "insert">> implements BaseRepository<S, S[ID]> {


protected constructor(protected readonly db: DbConnection,
protected readonly schema: M,
protected readonly primaryKey: ID) {
}


async create(data: S): Promise<S> {
await this.db.insert(this.schema).values([{ ...data }]);

const result = await this.db.select().from(this.schema).where(eq(this.schema[this.primaryKey], "LAST_INSERT_ID()"));
return result[0];
}

async delete(id: S[ID]): Promise<void> {
const result = await this.db.delete(this.schema).where(eq(this.schema[this.primaryKey], id));
}

async findAll(): Promise<S[]> {
return this.db.select().from(this.schema);
}

findOne(id: S[ID]): Promise<S | null> {
return this.db.select().from(this.schema).where(eq(this.schema[this.primaryKey], id));
}

async update(id: S[ID], data: I): Promise<S | null> {
const entity = this.findOne(id);
if (!entity) {
throw new Error("Entity not found");
}
this.db.update(this.schema).set(data).where(eq(this.schema[this.primaryKey], id));
return this.findOne(id);
}

}
34 Replies
Ayaz
AyazOP15mo ago
Angelelz
Angelelz15mo ago
Right now your Generic S and your Generic ID parameters don't have any relation The first thing you should do is in the first line:
export interface BaseRepository<T, ID extends keyof T> ...
...
export interface BaseRepository<T, ID extends keyof T> ...
...
Do your tables have a common name of the primary key, like id? That would make it a lot easier, with less generic parameters
Ayaz
AyazOP15mo ago
Usually yes You can check in the file how I’m implementing it I was thinking of passing the primary key in the generic
Angelelz
Angelelz15mo ago
If you do:
export abstract class CrudRepository<
M extends MySqlTable & { id: string },
...
export abstract class CrudRepository<
M extends MySqlTable & { id: string },
...
If your id is a string I might be wrong, but that would make not necessary to have a generic for the id Because you know in all your schemas have an id: string BTW, What errors are you seeing that made ask for help?
Ayaz
AyazOP15mo ago
No description
Ayaz
AyazOP15mo ago
No description
Ayaz
AyazOP15mo ago
No description
Ayaz
AyazOP15mo ago
No description
Ayaz
AyazOP15mo ago
I ended up going this route
Ayaz
AyazOP15mo ago
No description
Ayaz
AyazOP15mo ago
it is working but I'm on finding a way to add select criteria or I might just keep it as is and just make per repository queries @angelelz what do you think? 😄
Angelelz
Angelelz15mo ago
Interesting, I didn't have time this weekend to test it.
Ayaz
AyazOP15mo ago
this is how you extend it
No description
Ayaz
AyazOP15mo ago
not the best looking but works 😄
Angelelz
Angelelz15mo ago
Looks great, mr. typescript wizard!
Jukka Tawast
Jukka Tawast15mo ago
Did you manage to do this in a way that would allow to pass columns as a parameter for select() and the types for the returned rows for find/findAll would be correct? I got it to a point where all the real db columns were right but it was missing the "virtual" ones. So
db.select({
id: table.id,
madeUpFoo: sql<string>`lower(${table.bar})`
})...
db.select({
id: table.id,
madeUpFoo: sql<string>`lower(${table.bar})`
})...
was missing the madeUpFoo from the return type.
Angelelz
Angelelz15mo ago
Just my humble opinion, my 2 cents so to speak. I don't really like this pattern, I prefer to have my data model behind an interface. The types are really easy to implement, as the interface wouldn't care about implementation details, just return types. Then create classes that implement that interface, and instantiate it with the db object. Abstract classes are good for library code, where you need several implementation classes to have an underlying shared API and methods. In application code, when you create a record, you might need to also create related records. When you need to get records, you'll also likely need to join them with related records, etc. That's where this type of Crud abstract classes fall apart, imo. Interfaces also make your code easy to test and replacing implementation classes becomes a breeze
Jukka Tawast
Jukka Tawast15mo ago
Yeah, I do see what you mean. Personally I wasn't even planning to use the repositories for the more difficult queries/stuff for the reasons you just wrote, which in turn would make them kinda unnecessary I guess
Angelelz
Angelelz15mo ago
It's an interesting exercise, not gonna lie! And I wouldn't like my opinion to discourage anyone from attempting to implement it and probably discover some good benefits!
Jukka Tawast
Jukka Tawast15mo ago
Well exercise it was indeed. And I lost 😄 But indeed you kinda have to learn quite a lot of a library when you're trying to do generic stuff...
DiamondDragon
DiamondDragon15mo ago
@Angelelz @Ayaz this was my attempt at implementing this pattern... any thoughts/suggestions? @Raphaël M (@rphlmr) ⚡ i also saw you commented on this somewhere else. edit: my transaction was setup wrong but this seems to work
private getQueryExecutor(tx?: Db): Db {
return tx || db
}

public constructor(
protected table: TTable,
protected pk: TKeys,
protected tx?: TransactionsDb,
) {}
....
async delete(
condition: SQL,
tx?: TransactionsDb,
): Promise<InferSelectModel<TTable>[]> {
const queryExecutor = this.getQueryExecutor(tx)

return await queryExecutor
.delete(this.table)
.where(condition)
.returning()
}

async transaction<T>(
transactionCallback: (tx: TransactionsDb) => Promise<T>,
config?: PgTransactionConfig,
): Promise<T> {
return db.transaction(async tx => {
return transactionCallback(tx)
}, config)
}
private getQueryExecutor(tx?: Db): Db {
return tx || db
}

public constructor(
protected table: TTable,
protected pk: TKeys,
protected tx?: TransactionsDb,
) {}
....
async delete(
condition: SQL,
tx?: TransactionsDb,
): Promise<InferSelectModel<TTable>[]> {
const queryExecutor = this.getQueryExecutor(tx)

return await queryExecutor
.delete(this.table)
.where(condition)
.returning()
}

async transaction<T>(
transactionCallback: (tx: TransactionsDb) => Promise<T>,
config?: PgTransactionConfig,
): Promise<T> {
return db.transaction(async tx => {
return transactionCallback(tx)
}, config)
}
DiamondDragon
DiamondDragon15mo ago
@Andrew Sherman i saw you also recommended this service-repo pattern somewhere else. any suggestions? I was trying to add joins but was getting errors i think on the return type... not sure how to handle joins. Maybe it's too complicated and i should just handle this in other repos that implement BaseRepo?
interface ISelectOptions<TSelection> {
where?: SQL
joins?: Join[]
}

// TODO: Add joins
async read(
options?: ISelectOptions<InferInsertModel<TTable>>,
tx?: TransactionsDb,
): Promise<InferSelectModel<TTable>[]> {
const queryExecutor = this.getQueryExecutor(tx)

let query = queryExecutor.select().from(this.table)

if (options?.where) {
query = query.where(options.where)
}

...
if (options?.joins) {
for (const join of options.joins) {
switch (join.joinType) {
case 'inner':
query = query.innerJoin(join.table, join.on)
break
case 'left':
query = query.leftJoin(join.table, join.on)
break
case 'right':
query = query.rightJoin(join.table, join.on)
break
case 'full':
query = query.fullJoin(join.table, join.on)
break
}
}
}

return (await query) as InferSelectModel<TTable>[]
}
interface ISelectOptions<TSelection> {
where?: SQL
joins?: Join[]
}

// TODO: Add joins
async read(
options?: ISelectOptions<InferInsertModel<TTable>>,
tx?: TransactionsDb,
): Promise<InferSelectModel<TTable>[]> {
const queryExecutor = this.getQueryExecutor(tx)

let query = queryExecutor.select().from(this.table)

if (options?.where) {
query = query.where(options.where)
}

...
if (options?.joins) {
for (const join of options.joins) {
switch (join.joinType) {
case 'inner':
query = query.innerJoin(join.table, join.on)
break
case 'left':
query = query.leftJoin(join.table, join.on)
break
case 'right':
query = query.rightJoin(join.table, join.on)
break
case 'full':
query = query.fullJoin(join.table, join.on)
break
}
}
}

return (await query) as InferSelectModel<TTable>[]
}
Andrii Sherman
Andrii Sherman15mo ago
Can you share a real-world use case where you need to add joins to a query based on specific external values passed from the client?
DiamondDragon
DiamondDragon15mo ago
Trying to design my app with Airtable like query/filter capabilities across other relational data... so user could filter on a "Contacts" object by applying filters on related data like a "Tags" object, filtering for "Tags that have conditions A, D, F, and E". I could totally be approaching this completely wrong tho
Andrii Sherman
Andrii Sherman15mo ago
good, this scenario makes sense for me @alexblokh @bloberenober I guess this is a good example for conditional joins
DiamondDragon
DiamondDragon15mo ago
Yeah, this is the kind of query capabilities I'd love to build on the front end. Love the power& use cases it enables on the user side
No description
Kuba
Kuba10mo ago
Was something changed in the meantime how sql behaves since this time? I tried implementing this pattern, but I'm facing a strange problem: - when I write sql`id` a result is found without a problem, - when I try interpolating id string in this manner: sql`${this.primaryKey}` I get 0 results from the query. (The id string is passed to the class as an argument and is saved to a field.) I ensured that the primaryKey has "id" string and I cannot understand why it I get 0 results when interpolating it @Andrii Sherman So in the nutshell, this works:
.where(eq(sql`id`, id));
.where(eq(sql`id`, id));
but this doesn't:
.where(eq(sql`${"id"}`, id));
.where(eq(sql`${"id"}`, id));
Inspecting this further with a debugger results that these are indeed different. The difference is in the queryChunks attribute:
// 1 chunk: StringChunk
const sqlRegular = sql`id`;

// 3 chunks: StringChunk, "id", String chunk
const value = "id";
const sqlInterpolated = sql`${value}`;
// 1 chunk: StringChunk
const sqlRegular = sql`id`;

// 3 chunks: StringChunk, "id", String chunk
const value = "id";
const sqlInterpolated = sql`${value}`;
Kuba
Kuba10mo ago
No description
Angelelz
Angelelz10mo ago
This is correct behavior. The SQL operator will take any dynamic parameter you pass, and put it in a parameters array to avoid SQL injection
Kuba
Kuba10mo ago
Exactly. I've read up on this on docs, as I was just curious why it isn't working (I assumed it worked in the code sent by @Ayaz ). As sql operator produces parametrized queries, now I understand that this evaluates to WHERE ('id' = 'some-id') , so this is always false and returns no results. sql.raw() is what I needed, as it doesn't escape the string pased to it. https://orm.drizzle.team/docs/sql#sqlraw
Drizzle ORM - next gen TypeScript ORM
Drizzle ORM is a lightweight and performant TypeScript ORM with developer experience in mind.
minester16
minester1610mo ago
@Ayaz , that's an awesome example. And really helped me. Do you have github address which I can browse? I really like how you code and wanna inspect your proejcts and learn from them.
Ayaz
AyazOP10mo ago
Hi @minester16 , sadly, I don't really have open projects to highlight these stuff, most of them are on the job.
Kuba
Kuba8mo ago
If anyone would need to add custom ordering to this pattern, take a look at https://discord.com/channels/1043890932593987624/1217899465508257893 If anyone needs to extend the above with upserts, there you go:
async upsert(
data: InferInsertModel<TTable> & PgUpdateSetSource<TTable>,
): Promise<InferSelectModel<TTable>> {
const upsertRes = await this.database
.insert(this.table)
.values(data)
.onConflictDoUpdate({
target: { name: "id" } as unknown as IndexColumn,
set: data,
})
.returning();

return upsertRes[0] as InferSelectModel<TTable>;
}
async upsert(
data: InferInsertModel<TTable> & PgUpdateSetSource<TTable>,
): Promise<InferSelectModel<TTable>> {
const upsertRes = await this.database
.insert(this.table)
.values(data)
.onConflictDoUpdate({
target: { name: "id" } as unknown as IndexColumn,
set: data,
})
.returning();

return upsertRes[0] as InferSelectModel<TTable>;
}
Kuba
Kuba8mo ago
The name attribute comes from this line of code
GitHub
drizzle-orm/drizzle-orm/src/pg-core/query-builders/insert.ts at 470...
Headless TypeScript ORM with a head. Runs on Node, Bun and Deno. Lives on the Edge and yes, it's a JavaScript ORM too 😅 - drizzle-team/drizzle-orm
Want results from more Discord servers?
Add your server