drizzle-team / drizzle-orm

Headless TypeScript ORM with a head. Runs on Node, Bun and Deno. Lives on the Edge and yes, it's a JavaScript ORM too 😅
https://orm.drizzle.team
Apache License 2.0
24.53k stars 643 forks source link

[BUG]: Single-use generic type inferencing for libsql batch response #1301

Open Artemoire opened 1 year ago

Artemoire commented 1 year ago

What version of drizzle-orm are you using?

0.28.6

What version of drizzle-kit are you using?

0.19.13

Describe the Bug

When using a batch query which maps data into queries, I have to use typescript kung fu to make sure batch API does not complain.

Example use-case:

// ... database intialization
const db: LibSQLDatabase<Record<string, unknown>> = drizzle(...);
// ... batch api usage
const queries = transformedDataArray.map(data=>
  db.insert(schemas.transformData)
  .values(data)
  .onConflictDoUpdate({
    target: [schemas.transformData.key1, schemas.transformData.key2],
    set: { value: data.value }
  })
);

type Query = typeof queries[number];

await db.batch(
  queries as [Query, ...Query[]] // typescript kung fu required to make it work
);

Expected behavior

I would expect the batch response generic definition to allow using dynamic arrays instead of only Readonly tuples.

I think the issue is with how BatchResponse is defined: https://github.com/drizzle-team/drizzle-orm/blob/main/drizzle-orm/src/libsql/driver.ts#L34

TQuery extends Readonly<[U, ...U[]]> allows the batch parameter to only be a const tuple. I'm not an expert but a simple solution would be allowing TQuery to either extend a tuple or an array like so: TQuery extends Readonly<[U, ...U[]]> | U[]

Another potential solution would be to allow sending nested array queries, but that would incur runtime costs

Environment & setup

In my local vscode typescript server

wesleycoder commented 8 months ago

Fyi, this also spreads to astro:db, as mentioned in withastro/astro#10462.

bkyerv commented 7 months ago

also struggling with this issue

L-Mario564 commented 1 month ago

Pretty sure this issue isn't driver specific. There's currently not much priority towards the batch API, but we'll certainly address this at some point.