-
-
Notifications
You must be signed in to change notification settings - Fork 645
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
🔧 upgrade better-sqlite3 #524
Conversation
9cf884a
to
b5da47a
Compare
WalkthroughThe pull request involves an update to the Tip CodeRabbit's docstrings feature is now available as part of our Early Access Program! Simply use the command 📜 Recent review detailsConfiguration used: CodeRabbit UI ⛔ Files ignored due to path filters (2)
📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
upcoming-release-notes/524.md
is excluded by!**/*.md
yarn.lock
is excluded by!**/yarn.lock
,!**/*.lock
📒 Files selected for processing (1)
package.json
(1 hunks)
🔇 Additional comments (2)
package.json (2)
26-26
: Consider updating @types/better-sqlite3
The type definitions package might need to be updated to match the new better-sqlite3 version.
Also applies to: 44-44
Line range hint 67-69
: Consider updating the minimum Node.js version
Since this upgrade is specifically for better Node.js v22+ support, consider updating the engines
field to reflect the minimum supported Node.js version required by better-sqlite3 v11.
@@ -23,7 +23,7 @@ | |||
"@actual-app/crdt": "2.1.0", | |||
"@actual-app/web": "24.12.0", | |||
"bcrypt": "^5.1.1", | |||
"better-sqlite3": "^9.6.0", | |||
"better-sqlite3": "^11.7.0", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Codebase verification
Update @types/better-sqlite3 and verify Node.js compatibility
The codebase uses standard better-sqlite3 patterns (prepare, transaction, exec) that are stable across versions. However, two items need attention:
- The @types/better-sqlite3 package is at v7.6.7 while better-sqlite3 is being upgraded to v11.7.0. Update @types/better-sqlite3 to a compatible version (^9.0.0).
- Your package.json requires Node.js >= 18.0.0, which is compatible with better-sqlite3 v11.7.0.
Usage patterns in your codebase:
- Basic SQL operations (prepare, run, all)
- Transactions
- No usage of deprecated or removed features
🔗 Analysis chain
Verify compatibility with the major version upgrade
The upgrade from better-sqlite3 v9 to v11 involves breaking changes. Let's verify the compatibility:
Additionally, please review the breaking changes in the better-sqlite3 changelog to ensure your usage patterns are compatible.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check compatibility and breaking changes
# Check better-sqlite3's package.json for Node.js version requirements
curl -s https://registry.npmjs.org/better-sqlite3/11.7.0 | jq -r '.engines.node'
# Search for SQL-related code that might be affected by breaking changes
rg -l "\.prepare\(|\.transaction\(|\.pragma\(" --type js --type ts
# Check if there are any migration files that might need updates
fd -e sql
Length of output: 331
Script:
#!/bin/bash
# Let's check the actual SQL usage patterns in the files to verify compatibility
# Check db.js for database initialization and core patterns
echo "=== db.js ==="
rg -A 5 "\.prepare\(|\.transaction\(|\.pragma\(" src/db.js
# Check SQL patterns in migration file
echo -e "\n=== migrations/1719409568000-multiuser.js ==="
rg -A 5 "\.prepare\(|\.transaction\(|\.pragma\(" migrations/1719409568000-multiuser.js
# Check content of SQL file
echo -e "\n=== src/sql/messages.sql ==="
cat src/sql/messages.sql
# Check current @types/better-sqlite3 version
echo -e "\n=== @types/better-sqlite3 in package.json ==="
rg "@types/better-sqlite3" package.json
Length of output: 1242
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
types updated to 7.6.12. Version 9.0.0 was hallucinated.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
b5da47a
to
f2de20b
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for this!
I've been running it for a couple of days on my instance and it's been completely fine, there are no electron concerns here either because it's the server.
This version has better support for recent (v22+) Node versions.