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

feat(drizzle): add basic support for drizzle-orm postgres #179

Conversation

fjanicki
Copy link
Contributor

@fjanicki fjanicki commented Oct 10, 2024

I still need to add a test suite but this should cover the basics of what's needed for Drizzle-ORM

@fjanicki fjanicki mentioned this pull request Oct 10, 2024
@Papooch
Copy link
Owner

Papooch commented Oct 11, 2024

Thanks for the PR. I can see that you hard-coded only Postgres types. Do you think you could generalize it for any database that drizzle supports? In a similar manner that the Kysely adapter is implemented - using a generic argument for the database type. I am not sure to what extent is that possible, but I'd rather not lock the support for Postgres only.

@fjanicki
Copy link
Contributor Author

fjanicki commented Oct 11, 2024

I can take a look but this is the only db adapter I've been using in drizzle.

@Papooch
Copy link
Owner

Papooch commented Oct 11, 2024

I understand - that's why I encourage people to build their own specific adapters that cover their needs, because it is pretty straightforward.

But if this library should provide a ready-made adapter for a specific ORM / query builder, then it should not be tied to only a subset of (or just a single) supported databases.

@fjanicki
Copy link
Contributor Author

Yeah it makes sense. I'll look into how it's done with Kysely. Just a quick question about how to handle dependencies. Since drizzle requires the installation of the underlying library (pg, postgresjs, mongodb, etc...) would that be a peer dependency? I would prefer to avoid having to install all the libs when someone pulls this library and only use whatever they need. How do you handle this case in this kind of lib? I never had to do this before so i'm not 100% sure.

@Papooch
Copy link
Owner

Papooch commented Oct 11, 2024

I found a bit of free time and had a go at it myself: #180

At first, it was really challenging, because drizzle doesn't really provide a base type (or a common interface) for all the different client types - they're just a bare class.

But it turns out, you don't even need a base type - all that is needed is to know what the basic signature of the transaction method is and you're able to extract options out of it.

I don't want your effort to go unnoticed, so I'll go ahead and merge your PR to my branch and base my changes on top of yours, if you agree.

@Papooch Papooch changed the base branch from main to feat/transactional-adapter-drizzle-tmp October 27, 2024 10:54
@Papooch Papooch merged commit 61b7655 into Papooch:feat/transactional-adapter-drizzle-tmp Oct 27, 2024
2 checks passed
Papooch pushed a commit that referenced this pull request Oct 27, 2024
* feat(drizzle): add basic support for drizzle-orm postgres

* feat(drizzle): update drizzle-orm package.json description

* feat(drizzle): update drizzle-orm package.json description

* feat(drizzle): remove copied changes
Papooch added a commit that referenced this pull request Oct 27, 2024
* feat(drizzle): add basic support for drizzle-orm postgres (#179)

* feat(drizzle): add basic support for drizzle-orm postgres

* feat(drizzle): update drizzle-orm package.json description

* feat(drizzle): update drizzle-orm package.json description

* feat(drizzle): remove copied changes

* feat(transactional-adapter-drizzle-orm): make drizzle orm adapter universal

* build: add tsconfig reference

* docs: add docs for Drizzle ORM adapter

---------

Co-authored-by: Frederic Portaria-Janicki <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants