search

kysely-org / kysely

A type-safe typescript SQL query builder
https://kysely.dev
MIT License
9.82k stars 250 forks source link

RuntimeDriver is not exported #998

Closed bn-bgonzales closed 3 weeks ago

bn-bgonzales commented 1 month ago

I'm trying to initialize Kysely using KyselyProps and an executor has a dependency of RuntimeDriver, but it is not currently exported and I would like to avoid c/p this class in order to override some functionality in the base executor.

Can RuntimeDriver be exported?

igalklebanov commented 1 month ago

Hey 👋

Why are you trying to instantiate Kysely with KyselyProps? These are internals. Did you follow kysely.dev/docs/getting-started ?

@koskimas maybe we should alter the output types post-build to remove KyselyProps from being an option in userland?

bn-bgonzales commented 1 month ago

Hi @igalklebanov

I wrote an Audit plugin that audits row data for all create/update/delete queries, but this block of code restricts changing node.kind in transformQuery().

https://github.com/kysely-org/kysely/blob/master/src/query-executor/query-executor-base.ts#L37

I am currently doing this as there isn't an async callback called before a query is executed in order to track deletes. I overrode this logic by instantiating Kysely with KyselyProps I found in another github issue and noticed that RuntimeDriver dependency isn't exported.

I needed to transform a delete query into a select, so that I can audit data that is being deleted before the delete query is ran, in a generic way (applied to all deletes ran).

The node.kind check makes sense, but I have to override it for this functionality, unless I put a PR up to add a plugin callback that is triggered before a query is executed with the current database connection. Then I would not need to export this RuntimeDriver that I noticed was missing.

Thoughts?

koskimas commented 1 month ago

You shouldn't convert queries into other queries inside Kysely. Kysely is a query builder, and if you build a delete query, it should build a delete query.

As @igalklebanov said, the KyselyProps constructor and RuntimeDriver are part of the internal API and shouldn't be used/exposed.

We should remove this check though as it's too prohibitive.

igalklebanov commented 1 month ago

We should remove this check though as it's too prohibitive.

Maybe make this configurable and keep it strict by default? As a way to defend from "bad faith plugins"..