Skip to content

Add custom Introspector class to allow transactions#24

Open
samlaycock wants to merge 5 commits intoaidenwallis:mainfrom
samlaycock:main
Open

Add custom Introspector class to allow transactions#24
samlaycock wants to merge 5 commits intoaidenwallis:mainfrom
samlaycock:main

Conversation

@samlaycock
Copy link

  • I have verified my changes didn't break the example project.

Description

This dialect used the built-in Kysely SqliteIntrospector class as its introspector, which under the hood makes queries to the sqlite_master table, which in D1 is a no go - I've extended SqliteIntrospector into a D1Introspector to support using the table_list and table_info PRAGMAs to query table metadata instead.

Also, I've bumped up the version of kysely to the latest while I'm here as there were type discrepancies between 0.23.3 and 0.26.3.

@aidenwallis
Copy link
Owner

aidenwallis commented May 5, 2024

Would this be a breaking change? (Because of the kysely-d1 version bump?)

@atinux
Copy link

atinux commented Oct 17, 2024

I can confirm that this PR actually fixes an issue to run migrations with better-auth and Cloudflare D1.

I don't think it's a breaking change but actually a fix, the kysely version difference is only in dev dependencies here.

Any chance to see this merged and release @aidenwallis ? 🙏

PS: thank you for making this great adapter ❤️

@samlaycock
Copy link
Author

Sorry @aidenwallis, completely missed your response. Can confirm the version bump isn't a breaking change, it's only to have the correct typescript types.

@waynesbrain
Copy link

Hi, this does not allow SQL transactions though, it allows migrations. Is that correct?

@waynesbrain
Copy link

Has anyone put together a fork with some of these pull requests included? I had to fork this to an internal lib because the ESM compat pull request was never released to NPM and it was causing the whole CJS release of Kysely to be bundled.

I need this pull, the ESM compatibility, the API dialect and the batch helper pull requests.

@jcramb
Copy link

jcramb commented Apr 22, 2025

Any update on this getting merged - I'm also in need of these fixes.

@joshualyon
Copy link

@aidenwallis is there anything preventing this from being merged that we, the community, can help with?

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.

6 participants