[TypeScript] Make db.first generic to make it easy to type DB query results
#4248
Conversation
actual-github-bot
bot
changed the title
db.first generic to make it easy to type DB query resultsdb.first generic to make it easy to type DB query results
✅ 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
View detailed bundle breakdownAdded No assets were added Removed No assets were removed Bigger
Smaller No assets were smaller Unchanged No assets were unchanged |
|
👋 Hi! It looks like this PR has not had any changes for a week now. Would you like someone to review this PR? If so - please remove the "[WIP]" prefix from the PR title. That will let the community know that this PR is open for a review. |
|
This PR was closed because it has been stalled for 5 days with no activity. |
joel-jeremy
changed the title
db.first generic to make it easy to type DB query resultsdb.first generic to make it easy to type DB query results
joel-jeremy
force-pushed
the
ts-db-first
branch
from
bd930e7 to
4422e02
Compare
WalkthroughThis pull request introduces extensive type safety improvements across the codebase by enhancing how database query functions are utilized. Numerous updates add explicit type annotations and generic parameters to methods such as Possibly related PRs
Suggested labels
Suggested reviewers
📜 Recent review detailsConfiguration used: CodeRabbit UI ⛔ Files ignored due to path filters (2)
📒 Files selected for processing (31)
💤 Files with no reviewable changes (1)
🧰 Additional context used🧠 Learnings (2)packages/loot-core/src/server/accounts/sync.ts (1)packages/loot-core/src/server/accounts/transaction-rules.test.ts (1)🔇 Additional comments (66)
✨ Finishing Touches
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.
Caution
Inline review comments failed to post. This is likely due to GitHub's limits when posting large numbers of comments.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
packages/loot-core/src/mocks/budget.ts (1)
413-414:⚠️ Potential issueRemove incorrect starting_balance_flag.
The
starting_balance_flagshould only be set for the initial balance transaction, not for regular transactions.- starting_balance_flag: true,
🧹 Nitpick comments (10)
packages/loot-core/src/server/db/index.ts (2)
179-183: Consider extending generic support tofirstSync.
For consistency, adding a<T>parameter (similar to the async version) would eliminate many manual casts.
198-202: Adding a generic signature would improve maintainability.
Currently returningas anymight lead to subtle errors if usage is unclear.packages/loot-core/src/server/accounts/link.ts (1)
7-10: Include 'name' in the returned type or remove it from the query.
You currently fetchid, bank_id, namebut your pick only declares'id' | 'bank_id'. If you plan to use or storename, add it to the pick.packages/loot-core/src/server/accounts/payees.ts (2)
4-17: Add explicit return type tocreatePayeefunction.The function's return type should be explicitly defined for better type safety.
-export async function createPayee(description) { +export async function createPayee(description): Promise<number> {The type parameter
Pick<db.DbPayee, 'id'>correctly specifies that only the 'id' field is needed from the query result.
19-37: Add explicit return type togetStartingBalancePayeefunction.The function's return type should be explicitly defined for better type safety.
-export async function getStartingBalancePayee() { +export async function getStartingBalancePayee(): Promise<{ + id: number; + category: number | null; +}> {The type parameter
db.DbCategorycorrectly specifies the expected shape of the query results.packages/loot-core/src/server/migrate/migrations.test.ts (1)
65-67: LGTM! Consider extracting the type for reuse.The type parameter
{ sql: string }correctly specifies the shape of the query result. Consider extracting this type to a constant for reuse across both queries.type SqliteTableInfo = { sql: string }; // Then use it in both queries: let desc = await db.first<SqliteTableInfo>(...);Also applies to: 72-74
packages/loot-core/src/server/db/types/index.ts (1)
229-229: Fix typo in field name.The field name
show_uncateogorizedcontains a typo. It should beshow_uncategorized.- show_uncateogorized: 1 | 0; + show_uncategorized: 1 | 0;packages/loot-core/src/server/schedules/find-schedules.ts (1)
340-343: LGTM! Type safety enhancement for transaction retrieval, with a suggestion.The type parameter ensures type safety for the transaction view query result.
Consider optimizing the query by selecting only the required fields instead of all columns (
SELECT *). This would improve performance and reduce memory usage.- 'SELECT * FROM v_transactions WHERE account = ? AND parent_id IS NULL ORDER BY date DESC LIMIT 1', + 'SELECT date FROM v_transactions WHERE account = ? AND parent_id IS NULL ORDER BY date DESC LIMIT 1',packages/loot-core/src/mocks/budget.ts (2)
1-1: Consider enabling TypeScript strict mode.The file currently disables strict mode with
// @ts-strict-ignore. Consider enabling it to leverage TypeScript's type safety features fully.-// @ts-strict-ignore +// TODO: Fix type errors and remove ts-strict-ignore
85-255: Consider refactoring test data generation.The account filling functions share similar patterns and could benefit from:
- Extracting common logic into reusable helper functions
- Converting magic numbers into named constants for better maintainability
Example refactor for magic numbers:
const CONSTANTS = { INCOME_PROBABILITY: { PRIMARY_CHECKING: 0.09, CHECKING: 0.04, SAVINGS: 0.3 }, AMOUNT_RANGES: { INCOME: { MIN: 50000, MAX: 70000 }, EXPENSE: { MIN: 0, MAX: -700 } } };Also applies to: 257-303, 305-339, 341-384, 386-421, 423-459
🛑 Comments failed to post (1)
packages/loot-core/src/mocks/budget.ts (1)
462-462:
⚠️ Potential issueFix comparison operators in conditions.
Using assignment operators (=) instead of comparison operators (===) in conditions can lead to unexpected behavior.
- const primaryAccount = accounts.find(a => (a.name = 'Bank of America')); + const primaryAccount = accounts.find(a => a.name === 'Bank of America');Also applies to: 757-757
Follow up to #4247
db.runQuerydb.firstdb.firstSyncdb.alldb.selectdb.selectWithSchemadb.selectFirstWithSchema