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

Support for better-sqlite3 Driver #4871

Merged
merged 13 commits into from
Dec 9, 2021
Merged

Support for better-sqlite3 Driver #4871

merged 13 commits into from
Dec 9, 2021

Conversation

benjdlambert
Copy link
Contributor

With node-sqlite3 no longer being maintained, we want to have the ability to support a sqlite driver that is. https://github.com/JoshuaWise/better-sqlite3 seems to be a great replacement for node-sqlite3.

Still work in progress right now, that's why it's draft, but wanted to get some early feedback to make sure we're going the right way.

Closes #4511

benjdlambert and others added 8 commits November 29, 2021 13:44
…ter-sqlite3

Signed-off-by: blam <ben@blam.sh>
Signed-off-by: blam <ben@blam.sh>
…aking good progress!

Signed-off-by: blam <ben@blam.sh>
Signed-off-by: blam <ben@blam.sh>
Signed-off-by: blam <ben@blam.sh>
Signed-off-by: blam <ben@blam.sh>
@@ -174,6 +194,7 @@ const testConfigs = {
};

function getKnexForDb(db, configOverrides = {}) {
console.log(db);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this still helpful?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nop. 😂

@kibertoad
Copy link
Collaborator

Wow, this is super impressive! Great job

Signed-off-by: blam <ben@blam.sh>
Copy link
Collaborator

@OlivierCavadenti OlivierCavadenti left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems really cool work ! Do we need to flag sqlite3 as deprecated driver and show warnings that we will drop support ? I assume that in future we will switch to better-sqlite3 ?

test/unit/schema-builder/better-sqlite3.js Outdated Show resolved Hide resolved
test/unit/schema-builder/better-sqlite3.js Outdated Show resolved Hide resolved
test/util/db-helpers.js Outdated Show resolved Hide resolved
@kibertoad
Copy link
Collaborator

@OlivierCavadenti Considering the great job that vscode is doing on their sqlite fork, I don't think we need to deprecate dialect, just switch to vscode version (for which you already created PR, I believe)

@benjdlambert benjdlambert marked this pull request as ready for review December 3, 2021 10:07
@benjdlambert
Copy link
Contributor Author

Think this should be good to go now!

@kibertoad
Copy link
Collaborator

LGTM. Could you please also send a PR for documentation, expanding list of supported drivers?

@benjdlambert
Copy link
Contributor Author

@kibertoad yes of course, will round to that today!

@benjdlambert
Copy link
Contributor Author

Sorry - got sidetracked @kibertoad. Had a quick pass at updating some of the docs, hope that's what was needed :)

@kibertoad kibertoad merged commit 2bd1811 into knex:master Dec 9, 2021
@captainhusaynpenguin
Copy link

Looking forward to the release. Many thanks in advance!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add support for better-sqlite3
4 participants