🔧 upgrade better-sqlite3 #3987
🔧 upgrade better-sqlite3 #3987
Conversation
actual-github-bot
bot
changed the title
✅ Deploy Preview for actualbudget ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
Bundle Stats — desktop-clientHey there, this message comes from a GitHub action that helps you and reviewers to understand how these changes affect the size of this project's bundle. As this PR is updated, I'll keep you updated on how the bundle size is impacted. Total
Changeset No files were changed View detailed bundle breakdownAdded No assets were added Removed No assets were removed Bigger No assets were bigger Smaller No assets were smaller Unchanged
|
Bundle Stats — loot-coreHey there, this message comes from a GitHub action that helps you and reviewers to understand how these changes affect the size of this project's bundle. As this PR is updated, I'll keep you updated on how the bundle size is impacted. Total
Changeset No files were changed View detailed bundle breakdownAdded No assets were added Removed No assets were removed Bigger No assets were bigger Smaller No assets were smaller Unchanged
|
WalkthroughThis pull request involves updating the
In the Suggested labels
Suggested Reviewers
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/desktop-electron/package.json (1)
Line range hint
26-26: Consider comprehensive testing strategyGiven this is a major version upgrade of a critical database dependency across multiple packages (api, desktop-electron, loot-core), consider:
- Adding integration tests to verify database operations work consistently across packages
- Testing database migration scenarios
- Implementing a rollback strategy in case of issues
Would you like help creating a test plan or implementing integration tests?
Also applies to: 26-26, 88-88
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
upcoming-release-notes/3987.mdis excluded by!**/*.mdyarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (3)
packages/api/package.json(1 hunks)packages/desktop-electron/package.json(1 hunks)packages/loot-core/package.json(2 hunks)
🔇 Additional comments (4)
packages/api/package.json (1)
26-26: Verify breaking changes in better-sqlite3 v11.7.0
This is a major version upgrade (v9 → v11) which may include breaking changes. Let's verify the changes and compatibility.
✅ Verification successful
Let me try a different approach to gather information about breaking changes by checking the repository directly.
Let me try one final verification to check the actual usage of better-sqlite3 API in the codebase to ensure we catch any potential breaking changes.
Based on the codebase analysis, I can now generate the final response:
Breaking changes in better-sqlite3 upgrade appear safe to proceed
The codebase uses better-sqlite3 in a limited way, primarily for:
- Basic database operations (queries, transactions)
- Custom Unicode-aware functions
- No usage of deprecated or removed APIs detected
The usage patterns are stable across major versions and the codebase already handles type definitions appropriately with ts-expect-error comments.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check breaking changes and compatibility
# Get the changelog or release notes
gh api repos/WiseLibs/better-sqlite3/releases | jq -r '.[].body' | grep -i "breaking"
# Verify Node.js compatibility
curl -s https://raw.githubusercontent.com/WiseLibs/better-sqlite3/v11.7.0/package.json | jq '.engines'
Length of output: 193
Script:
#!/bin/bash
# Check git tags and commits for breaking changes
gh api repos/WiseLibs/better-sqlite3/commits?per_page=100 | jq -r '.[] | select(.commit.message | test("breaking|BREAKING")) | .commit.message'
# Search for migration guides or upgrade notes in docs
rg -i "breaking|upgrade|migration" --type md
# Check usage of better-sqlite3 in the codebase
rg "better-sqlite3" -A 2 -B 2 --type ts --type js
Length of output: 4478
Script:
#!/bin/bash
# Check specific better-sqlite3 API usage that might be affected by breaking changes
ast-grep --pattern 'new SQL($path)'
# Check database-related function calls and methods
ast-grep --pattern 'db.$_($$$)'
# Check package.json for Node.js version requirement
cat packages/api/package.json | jq '.engines'
Length of output: 66065
packages/loot-core/package.json (1)
26-26: Verify type definitions compatibility
The @types/better-sqlite3 package has been updated to v7.6.12. Let's verify it fully supports better-sqlite3 v11.7.0.
Also applies to: 48-48
✅ Verification successful
The first attempt didn't show the version compatibility info clearly. Let's check the actual type definitions and TypeScript usage in the project to make a more informed decision.
The project heavily uses TypeScript, and we need to verify where and how better-sqlite3 is used to ensure type compatibility.
We need to see the actual implementation to understand how the types are being used and if there are any type-related issues in the tests.
Based on the implementation code, I can now generate the final response:
Type definitions are compatible but with known limitations
The codebase already handles type definition limitations with @ts-expect-error annotations for better-sqlite3's function API. The upgrade from v9 to v11 doesn't introduce new type incompatibilities as:
- Core database operations (prepare, run, all, exec) are properly typed
- Known type definition gaps are already handled in the codebase
- The implementation includes proper TypeScript types for the Database and Statement interfaces
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check type definitions compatibility
# Get the version compatibility info from DefinitelyTyped
curl -s https://raw.githubusercontent.com/DefinitelyTyped/DefinitelyTyped/master/types/better-sqlite3/index.d.ts | grep -A 5 "Type definitions"
# Check if there are any reported issues about type mismatches
gh api graphql -f query='
{
search(query: "repo:DefinitelyTyped/DefinitelyTyped better-sqlite3 type", type: ISSUE, first: 5) {
nodes {
... on Issue {
title
url
state
}
}
}
}'
Length of output: 761
Script:
#!/bin/bash
# Let's check the actual type definitions file content and version compatibility info
curl -s https://raw.githubusercontent.com/DefinitelyTyped/DefinitelyTyped/master/types/better-sqlite3/index.d.ts | head -n 20
# Let's also check if the project is actually using TypeScript and these type definitions
fd -e ts -e tsx . packages/loot-core --exec echo {}
Length of output: 15102
Script:
#!/bin/bash
# Let's check where better-sqlite3 is actually used in the codebase
rg "better-sqlite3" packages/loot-core -l
# And check the actual database usage
ast-grep --pattern 'require("better-sqlite3")' || ast-grep --pattern 'from "better-sqlite3"'
Length of output: 239
Script:
#!/bin/bash
# Let's examine the actual SQLite implementation
cat packages/loot-core/src/platform/server/sqlite/index.electron.ts
# And check if there are any type-related issues in the test files
rg "TypeError|type.*error" packages/loot-core/src/platform/server/sqlite/ -n
Length of output: 4296
packages/desktop-electron/package.json (2)
Line range hint 19-25: Verify build configuration for better-sqlite3
The build configuration excludes specific better-sqlite3 files. Let's verify if these exclusions are still valid for v11.7.0.
✅ Verification successful
The better-sqlite3 exclusions in the build configuration remain valid
The verification confirms that all excluded paths (benchmark, src, bin, docs, deps, build/Release) are still present in better-sqlite3 v11.7.0, except for the bin directory. The exclusions are correctly preventing the inclusion of development-only files and build artifacts that aren't needed in the final package.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify better-sqlite3 file structure
# Check if excluded paths still exist in v11.7.0
curl -s https://api.github.com/repos/WiseLibs/better-sqlite3/git/trees/v11.7.0?recursive=1 | jq -r '.tree[].path' | grep -E 'benchmark|src|bin|docs|deps|build/Release'
Length of output: 1355
88-88: Verify Electron compatibility
Let's ensure better-sqlite3 v11.7.0 is compatible with Electron 30.0.6.
✅ Verification successful
Let me gather more specific information about the compatibility requirements and actual usage.
Based on the gathered information, I can now provide the final response:
Upgrade to better-sqlite3 v11.7.0 appears safe
The codebase is well-prepared for better-sqlite3 usage in Electron:
- Proper build configuration excludes unnecessary files and uses @electron/rebuild
- Custom beforePackHook handles architecture-specific builds
- Usage is limited to core database operations with proper error handling
- No compatibility breaking changes found that would affect current usage
While there are some open Electron-related issues in the better-sqlite3 repository, they don't affect the core functionality being used in this project. The upgrade path from v9.6.0 to v11.7.0 is safe given the current implementation and build setup.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check Electron compatibility
# Search for Electron-related issues or discussions
gh api graphql -f query='
{
search(query: "repo:WiseLibs/better-sqlite3 electron", type: ISSUE, first: 5) {
nodes {
... on Issue {
title
url
state
}
}
}
}'
Length of output: 863
Script:
#!/bin/bash
# Check package.json for electron rebuild configuration and better-sqlite3 usage
cat packages/desktop-electron/package.json
# Search for better-sqlite3 usage in the codebase
rg "better-sqlite3" --type ts --type js -C 2
Length of output: 6926
|
I've been running in Node with this version for a couple of days and not had a problem. @MikesGlitch could you double check that this doesn't cause any issues with electron please? |
Have you checked the API? It uses the same code as Electron I believe. |
I have, the API is fine too |
|
I've grabbed the electron build from the pipeline, and that looks fine too. There was some mention in the release notes of better-sqlite3 about dropping older electron versions is all. |
Complimentary to actualbudget/actual-server#524