Fix send types in a number of places (1/2)
#4146
Conversation
actual-github-bot
bot
changed the title
send types in a number of places (1/2)send types in a number of places (1/2)
✅ 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
View detailed bundle breakdownAdded No assets were added Removed No assets were removed 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
|
jfdoming
force-pushed
the
jfdoming/fix-send-types-1
branch
from
8139af6 to
efb74a9
Compare
jfdoming
changed the title
send types in a number of places (1/2)send types in a number of places (1/2)
WalkthroughThe pull request encompasses modifications across multiple components in the desktop client, focusing on enhancing error handling, type safety, and response processing. Changes span various files including Possibly related PRs
Suggested labels
Suggested reviewers
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.
Actionable comments posted: 3
🔭 Outside diff range comments (2)
packages/desktop-client/src/components/reports/spreadsheets/summary-spreadsheet.ts (1)
Line range hint
32-37: Type the response fromsendfunctionThe
sendfunction call lacks proper typing for its response, which could lead to runtime errors if the response structure changes.Add type information for the response:
interface MakeFiltersResponse { filters: FilterCondition[]; } const response = await send<MakeFiltersResponse>('make-filters-from-conditions', { conditions: conditions.filter(cond => !cond.customName), });packages/desktop-client/src/components/reports/spreadsheets/calendar-spreadsheet.ts (1)
Line range hint
39-46: Improve type safety of thesendresponseThe destructuring assignment lacks proper typing, and the response type should be consistent with other spreadsheet components.
Add proper typing to the response:
interface MakeFiltersResponse { filters: FilterCondition[]; } const { filters: filtersLocal } = await send<MakeFiltersResponse>( 'make-filters-from-conditions', { conditions: conditions.filter(cond => !cond.customName), }, );
🧹 Nitpick comments (4)
packages/desktop-client/src/components/reports/spreadsheets/calendar-spreadsheet.ts (2)
37-37: Declare and initializefiltersin one statementThe variable is declared but not initialized, which could lead to undefined behavior if the try-catch block fails.
Initialize the variable with an empty array:
-let filters: unknown[]; +let filters: FilterCondition[] = [];
Line range hint
39-48: Consider extracting filter creation to a shared utilityBoth spreadsheet components use similar logic to create filters from conditions. This could be extracted to reduce duplication and ensure consistent typing.
Create a shared utility function:
// utils/filters.ts export async function createFiltersFromConditions( conditions: RuleConditionEntity[] ): Promise<FilterCondition[]> { try { const { filters } = await send<MakeFiltersResponse>( 'make-filters-from-conditions', { conditions: conditions.filter(cond => !cond.customName), }, ); return filters; } catch (error) { console.error('Failed to make filters from conditions:', error); return []; } }packages/desktop-client/src/components/admin/UserDirectory/UserDirectory.tsx (1)
157-161: Improve type safety for response handling.While the error handling is more comprehensive, the string indexing could be improved with proper type definitions.
Consider defining a type for the response and using type guards:
type DeleteResponse = { error?: string; someDeletionsFailed?: boolean; }; const res = (await send('user-delete-all', [...selectedInst.items])) as DeleteResponse; const { error, someDeletionsFailed } = res;packages/desktop-client/src/components/modals/EditUser.tsx (1)
87-93: Consider using type-safe property access.The bracket notation
['error']and['id']could be replaced with more type-safe property access if the response type is properly defined. Consider adding type definitions for the response:type UserResponse = { error?: string; id?: string; };Then use type-safe property access:
- if (!res['error']) { - const newId = res['id']; + if (!res.error) { + const newId = res.id;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
upcoming-release-notes/4146.mdis excluded by!**/*.md
📒 Files selected for processing (10)
packages/desktop-client/src/components/ServerContext.tsx(2 hunks)packages/desktop-client/src/components/admin/UserAccess/UserAccess.tsx(1 hunks)packages/desktop-client/src/components/admin/UserDirectory/UserDirectory.tsx(2 hunks)packages/desktop-client/src/components/filters/SavedFilterMenuButton.tsx(1 hunks)packages/desktop-client/src/components/modals/CreateAccountModal.tsx(1 hunks)packages/desktop-client/src/components/modals/EditUser.tsx(1 hunks)packages/desktop-client/src/components/payees/ManagePayeesWithData.tsx(1 hunks)packages/desktop-client/src/components/reports/spreadsheets/calendar-spreadsheet.ts(1 hunks)packages/desktop-client/src/components/reports/spreadsheets/spending-spreadsheet.ts(1 hunks)packages/desktop-client/src/components/reports/spreadsheets/summary-spreadsheet.ts(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- packages/desktop-client/src/components/reports/spreadsheets/spending-spreadsheet.ts
🔇 Additional comments (7)
packages/desktop-client/src/components/filters/SavedFilterMenuButton.tsx (1)
45-45: LGTM! Type safety improvement.The explicit type annotation for the error state improves type safety and makes the expected error values clearer.
packages/desktop-client/src/components/admin/UserAccess/UserAccess.tsx (1)
109-111: LGTM! Robust null handling.The null coalescing operator with a properly structured default object prevents potential runtime errors when the server response is null or undefined.
packages/desktop-client/src/components/admin/UserDirectory/UserDirectory.tsx (2)
119-131: LGTM! Comprehensive error handling.The error handling implementation is robust, providing user-friendly notifications and proper loading state management.
136-136: LGTM! Complete dependency array.The dependency array now correctly includes all required dependencies, preventing potential stale closure issues.
packages/desktop-client/src/components/modals/CreateAccountModal.tsx (1)
91-91: LGTM! Good defensive programming practice.The nullish coalescing operator
??ensures safe iteration overresults.accountsby defaulting to an empty array if the value is null or undefined.packages/desktop-client/src/components/ServerContext.tsx (2)
96-100: LGTM! Good error handling practice.The null check ensures that we only set the server URL when we have a valid value, preventing potential issues with undefined server URLs.
142-143: Verify the non-null assertion is safe.The non-null assertion (
!) onserverURLassumes it will never be null at this point. Consider using a safer approach with explicit null checking or document why this assertion is safe.✅ Verification successful
The non-null assertion is safe in this context
The assertion is used after a successful
set-server-urlcall within anif (!error)block, guaranteeing that the subsequentget-server-urlwill return a value. The implementation inserver/main.tsconfirms this behavior.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check if there are any error handlers or conditions that ensure serverURL is never null # Look for error handling around set-server-url and get-server-url rg -A 5 "set-server-url|get-server-url" --type tsLength of output: 3243
packages/desktop-client/src/components/reports/spreadsheets/summary-spreadsheet.ts
Show resolved
Hide resolved
In #4145 I'm attempting to remove
anytypes from thesendfunction. In doing so I uncovered a bunch of places where types were set incorrectly. This PR cleans up those types (split into 2 for readability).