-
Notifications
You must be signed in to change notification settings - Fork 263
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
node-sqlite3 support #128
Comments
Why is it important for a driver to support N-API? I understand its appeal from the driver's maintainer's perspective (less work to maintain the interface) but why would we, the user's of the driver, care how it interacts with the native code? |
Usually there wouldn't and shouldn't be a good reason to. But sadly, Electron doesn't play as nicely as one would hope, and frequently requires one to rebuild modules with native dependencies. Another bit is trying to upgrade to the latest Electron version, and realizing that a library with a native dependency doesn't support the Node.js version of the latest Electron version yet. As I'm contributing to someone else's Electron application, I'd very much prefer choosing an option where they don't have to worry about such little ecosystem annoyances down the line. Hence my request. |
Ok gotcha. I don't think this is a good enough reason for me to add another sqlite driver to Kysely's core. Implementing a driver (a dialect) that uses node-sqlite yourself should be pretty straightforward. You could start by copying this dir and the only thing you need to change is the driver class, which is about 100 lines of code. Actually you could just import and use the other classes from kysely directly. |
Sounds good, then I'll close this issue. Glad to hear that it should be pretty easy to do 👍 |
Update for those who are curious: This was indeed quite easy to do. Thanks for making such a lovely library. |
Nice! Kysely exports |
Would it be possible to add support for node-sqlite3, since better-sqlite3 sadly lacks N-API support?
WiseLibs/better-sqlite3#271
The text was updated successfully, but these errors were encountered: