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

Convert sqlite3 to better-sqlite3 #210

Merged
merged 1 commit into from Jan 24, 2022
Merged

Convert sqlite3 to better-sqlite3 #210

merged 1 commit into from Jan 24, 2022

Conversation

claabs
Copy link
Contributor

@claabs claabs commented Jan 23, 2022

node-sqlite3 appears to be somewhat abandoned and has been racking up security vulnerabilities in its dependencies. This PR switches it to better-sqlite3 which is more performant, supports more architectures, and is actively maintained.

This PR also switches to using Bind Parameters which resolves an issue I was experiencing when storing values that contain a single quote (').

Note: I didn't really understand what was going on with the TestAdapter, so I just reverted these changes. The tests seem to still pass from what I see, although you may want to re-run manually to double check.

I understand this is a big change, so if you feel you want to stick with node-sqlite3 and just resolve its security vulnerabilities (like this solution), I can just port this module into a standalone package. However, you still may want to consider switching to bind parameters, which node-sqlite3 also offers.

- Resolve single quote parsing issue in query
@codecov
Copy link

codecov bot commented Jan 24, 2022

Codecov Report

Merging #210 (45a807a) into master (293ac0a) will decrease coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #210      +/-   ##
==========================================
- Coverage   99.74%   99.74%   -0.01%     
==========================================
  Files          11       11              
  Lines         399      390       -9     
==========================================
- Hits          398      389       -9     
  Misses          1        1              
Impacted Files Coverage Δ
packages/sqlite/src/index.js 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 293ac0a...45a807a. Read the comment docs.

@jaredwray jaredwray merged commit 33e1a60 into jaredwray:master Jan 24, 2022
@jaredwray
Copy link
Owner

@claabs - thanks so much for submitting this. The code looks great and we are moving it into the repo.

@claabs claabs deleted the better-sqlite3 branch January 24, 2022 21:03
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.

None yet

2 participants