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

Hooks for frequently made operations #2293

Merged
merged 32 commits into from
Feb 12, 2024
Merged

Hooks for frequently made operations #2293

merged 32 commits into from
Feb 12, 2024

Conversation

joel-jeremy
Copy link
Contributor

@joel-jeremy joel-jeremy commented Jan 27, 2024

Add hooks for frequently made operations in the codebase:

  • useAccounts
  • useAccount
  • useBudgetedAccounts
  • useOffBudgetAccounts
  • useClosedAccounts
  • useFailedAccounts
  • usePayees
  • usePayee
  • useLocalPrefs
  • useLocalPref
  • useGlobalPrefs
  • useGlobalPref
  • useDateFormat

Copy link

netlify bot commented Jan 27, 2024

Deploy Preview for actualbudget ready!

Name Link
🔨 Latest commit e352ee5
🔍 Latest deploy log https://app.netlify.com/sites/actualbudget/deploys/65c9393224308300087689e9
😎 Deploy Preview https://deploy-preview-2293.demo.actualbudget.org
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link
Contributor

github-actions bot commented Jan 27, 2024

Bundle Stats — desktop-client

Hey 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

Files count Total bundle size % Changed
10 4.43 MB → 4.44 MB (+15.74 kB) +0.35%
Changeset
File Δ Size
node_modules/react-redux/es/components/connectAdvanced.js 🆕 +8.91 kB 0 B → 8.91 kB
node_modules/react-redux/es/connect/connect.js 🆕 +4.38 kB 0 B → 4.38 kB
node_modules/react-redux/es/connect/selectorFactory.js 🆕 +3.26 kB 0 B → 3.26 kB
node_modules/react-redux/es/connect/wrapMapToProps.js 🆕 +1.33 kB 0 B → 1.33 kB
node_modules/react-redux/es/connect/mergeProps.js 🆕 +1.11 kB 0 B → 1.11 kB
src/components/sidebar/SidebarProvider.tsx 🆕 +832 B 0 B → 832 B
node_modules/react-redux/es/connect/mapDispatchToProps.js 🆕 +777 B 0 B → 777 B
node_modules/react-redux/es/utils/shallowEqual.js 🆕 +639 B 0 B → 639 B
node_modules/react-redux/es/utils/bindActionCreators.js 🆕 +439 B 0 B → 439 B
node_modules/react-redux/es/connect/mapStateToProps.js 🆕 +417 B 0 B → 417 B
src/hooks/useGlobalPref.ts 🆕 +363 B 0 B → 363 B
src/hooks/useLocalPref.ts 🆕 +351 B 0 B → 351 B
src/hooks/useAccounts.ts 🆕 +303 B 0 B → 303 B
src/hooks/usePayees.ts 🆕 +291 B 0 B → 291 B
src/hooks/useOffBudgetAccounts.ts 🆕 +205 B 0 B → 205 B
src/hooks/useBudgetedAccounts.ts 🆕 +204 B 0 B → 204 B
src/hooks/useClosedAccounts.ts 🆕 +175 B 0 B → 175 B
src/hooks/useAccount.ts 🆕 +161 B 0 B → 161 B
src/hooks/usePayee.ts 🆕 +151 B 0 B → 151 B
src/hooks/usePrivacyMode.ts 🆕 +108 B 0 B → 108 B
src/hooks/useUpdatedAccounts.ts 🆕 +97 B 0 B → 97 B
src/hooks/useFailedAccounts.ts 🆕 +95 B 0 B → 95 B
src/hooks/useDateFormat.ts 🆕 +92 B 0 B → 92 B
src/hooks/useLocalPrefs.ts 🆕 +80 B 0 B → 80 B
node_modules/hoist-non-react-statics/dist/hoist-non-react-statics.cjs.js 📈 +2.36 kB (+571.87%) 423 B → 2.78 kB
src/components/sidebar/Sidebar.tsx 📈 +2.75 kB (+92.92%) 2.96 kB → 5.72 kB
src/components/util/DisplayId.tsx 📈 +148 B (+20.76%) 713 B → 861 B
src/components/budget/BudgetCategories.jsx 📈 +858 B (+11.15%) 7.51 kB → 8.35 kB
home/runner/work/actual/actual/packages/loot-core/src/client/reducers/queries.ts 📈 +165 B (+6.89%) 2.34 kB → 2.5 kB
src/components/modals/ConfirmCategoryDelete.tsx 📈 +204 B (+5.21%) 3.82 kB → 4.02 kB
home/runner/work/actual/actual/packages/loot-core/src/client/actions/account.ts 📈 +170 B (+3.73%) 4.45 kB → 4.61 kB
src/components/modals/LoadBackup.jsx 📈 +122 B (+2.65%) 4.49 kB → 4.61 kB
src/style/theme.tsx 📈 +43 B (+2.39%) 1.75 kB → 1.8 kB
src/components/modals/CloseAccount.tsx 📈 +93 B (+1.38%) 6.6 kB → 6.69 kB
src/hooks/useCategories.ts 📈 +3 B (+0.96%) 312 B → 315 B
src/components/modals/SelectLinkedAccounts.jsx 📈 +54 B (+0.89%) 5.93 kB → 5.99 kB
src/components/accounts/Account.jsx 📈 +355 B (+0.87%) 40.06 kB → 40.41 kB
src/components/budget/BudgetTable.jsx 📈 +69 B (+0.86%) 7.83 kB → 7.89 kB
src/components/accounts/Header.jsx 📈 +30 B (+0.22%) 13.59 kB → 13.62 kB
src/components/settings/Reset.tsx 📈 +3 B (+0.13%) 2.2 kB → 2.2 kB
node_modules/@react-spring/core/dist/react-spring_core.modern.mjs 📈 +4 B (+0.01%) 58.12 kB → 58.12 kB
src/components/autocomplete/PayeeAutocomplete.tsx 📉 -6 B (-0.05%) 12.22 kB → 12.21 kB
src/components/autocomplete/AccountAutocomplete.tsx 📉 -6 B (-0.12%) 4.94 kB → 4.93 kB
src/components/ManageRules.tsx 📉 -11 B (-0.13%) 8.36 kB → 8.35 kB
src/components/select/DateSelect.tsx 📉 -13 B (-0.15%) 8.41 kB → 8.4 kB
src/components/modals/EditRule.jsx 📉 -55 B (-0.16%) 34.63 kB → 34.58 kB
src/components/modals/ImportTransactions.jsx 📉 -63 B (-0.17%) 37.14 kB → 37.08 kB
home/runner/work/actual/actual/packages/loot-core/src/client/query-hooks.tsx 📉 -4 B (-0.23%) 1.73 kB → 1.73 kB
src/components/schedules/ScheduleDetails.jsx 📉 -84 B (-0.30%) 27.65 kB → 27.57 kB
src/components/budget/MobileBudget.tsx 📉 -30 B (-0.36%) 8.16 kB → 8.13 kB
src/components/schedules/SchedulesTable.tsx 📉 -41 B (-0.40%) 10.02 kB → 9.98 kB
src/components/transactions/MobileTransaction.jsx 📉 -178 B (-0.42%) 41.21 kB → 41.03 kB
src/components/schedules/DiscoverSchedules.tsx 📉 -37 B (-0.58%) 6.27 kB → 6.23 kB
src/components/settings/Encryption.tsx 📉 -24 B (-0.60%) 3.89 kB → 3.86 kB
src/components/select/RecurringSchedulePicker.jsx 📉 -113 B (-0.66%) 16.71 kB → 16.6 kB
src/components/filters/FiltersMenu.jsx 📉 -87 B (-0.70%) 12.1 kB → 12.01 kB
src/components/modals/MergeUnusedPayees.jsx 📉 -48 B (-0.74%) 6.32 kB → 6.27 kB
src/components/accounts/AccountSyncCheck.jsx 📉 -33 B (-0.75%) 4.29 kB → 4.26 kB
src/components/reports/reports/NetWorth.jsx 📉 -33 B (-0.76%) 4.23 kB → 4.2 kB
src/components/util/GenericInput.jsx 📉 -37 B (-0.80%) 4.53 kB → 4.49 kB
src/components/payees/ManagePayeesWithData.jsx 📉 -33 B (-0.83%) 3.88 kB → 3.85 kB
src/components/budget/MobileBudgetTable.jsx 📉 -312 B (-0.84%) 36.48 kB → 36.17 kB
src/components/accounts/MobileAccounts.jsx 📉 -68 B (-0.90%) 7.39 kB → 7.32 kB
src/components/settings/Experimental.tsx 📉 -30 B (-0.93%) 3.14 kB → 3.11 kB
src/components/reports/reports/CustomReport.jsx 📉 -156 B (-1.07%) 14.3 kB → 14.15 kB
src/components/accounts/MobileAccount.jsx 📉 -71 B (-1.22%) 5.68 kB → 5.61 kB
src/components/modals/SwitchBudgetType.tsx 📉 -24 B (-1.44%) 1.63 kB → 1.61 kB
src/components/modals/EditField.jsx 📉 -103 B (-1.48%) 6.8 kB → 6.7 kB
src/components/FinancesApp.tsx 📉 -157 B (-1.49%) 10.3 kB → 10.15 kB
src/components/settings/index.tsx 📉 -96 B (-1.57%) 5.97 kB → 5.88 kB
src/components/settings/Format.tsx 📉 -87 B (-1.62%) 5.24 kB → 5.16 kB
src/components/transactions/SimpleTransactionsTable.jsx 📉 -115 B (-1.76%) 6.39 kB → 6.28 kB
src/components/Titlebar.tsx 📉 -218 B (-1.78%) 11.98 kB → 11.77 kB
src/components/rules/Value.tsx 📉 -103 B (-2.09%) 4.82 kB → 4.72 kB
src/components/settings/Export.tsx 📉 -48 B (-2.12%) 2.21 kB → 2.17 kB
src/components/reports/Overview.jsx 📉 -33 B (-2.21%) 1.46 kB → 1.43 kB
src/components/MobileWebMessage.tsx 📉 -112 B (-3.67%) 2.98 kB → 2.87 kB
src/components/ThemeSelector.tsx 📉 -60 B (-4.80%) 1.22 kB → 1.16 kB
src/components/App.tsx 📉 -237 B (-5.21%) 4.44 kB → 4.21 kB
src/components/budget/DynamicBudgetTable.tsx 📉 -118 B (-5.42%) 2.13 kB → 2.01 kB
src/components/Modals.tsx 📉 -514 B (-5.88%) 8.54 kB → 8.04 kB
src/components/rules/ScheduleValue.tsx 📉 -33 B (-6.55%) 504 B → 471 B
src/components/budget/rollover/RolloverContext.tsx 📉 -38 B (-7.12%) 534 B → 496 B
src/components/sidebar/Accounts.tsx 📉 -276 B (-7.34%) 3.67 kB → 3.4 kB
src/components/settings/Themes.tsx 📉 -66 B (-7.53%) 877 B → 811 B
src/components/budget/index.tsx 📉 -3.55 kB (-27.91%) 12.71 kB → 9.16 kB
src/components/sidebar/index.tsx 📉 -883 B (-40.80%) 2.11 kB → 1.25 kB
src/components/sidebar/SidebarWithData.tsx 🔥 -4.25 kB (-100%) 4.25 kB → 0 B
home/runner/work/actual/actual/packages/loot-core/src/shared/categories.ts 🔥 -2.49 kB (-100%) 2.49 kB → 0 B
home/runner/work/actual/actual/packages/loot-core/src/client/data-hooks/accounts.tsx 🔥 -631 B (-100%) 631 B → 0 B
home/runner/work/actual/actual/packages/loot-core/src/client/data-hooks/payees.tsx 🔥 -611 B (-100%) 611 B → 0 B
home/runner/work/actual/actual/packages/loot-core/src/client/privacy.ts 🔥 -109 B (-100%) 109 B → 0 B
View detailed bundle breakdown

Added

No assets were added

Removed

No assets were removed

Bigger

Asset File Size % Changed
static/js/wide.js 242.14 kB → 258.43 kB (+16.29 kB) +6.73%
static/js/index.js 2.73 MB → 2.73 MB (+232 B) +0.01%

Smaller

Asset File Size % Changed
static/js/narrow.js 80.3 kB → 79.83 kB (-481 B) -0.58%
static/js/ReportRouter.js 1.2 MB → 1.2 MB (-222 B) -0.02%
static/js/AppliedFilters.js 28.99 kB → 28.91 kB (-87 B) -0.29%

Unchanged

Asset File Size % Changed
static/js/indexeddb-main-thread-worker-e59fee74.js 13.5 kB 0%
static/js/resize-observer.js 18.37 kB 0%
static/js/ButtonLink.js 379 B 0%
static/js/BackgroundImage.js 122.29 kB 0%
static/js/BalanceTooltip.js 6.06 kB 0%

Copy link
Contributor

github-actions bot commented Jan 27, 2024

Bundle Stats — loot-core

Hey 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

Files count Total bundle size % Changed
1 1.2 MB 0%

Changeset

No files were changed

View detailed bundle breakdown

Added

No assets were added

Removed

No assets were removed

Bigger

No assets were bigger

Smaller

No assets were smaller

Unchanged

Asset File Size % Changed
kcab.worker.js 1.2 MB 0%

Copy link
Member

@MatissJanis MatissJanis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very exciting! This clean-up was long overdue. Thanks for undertaking it.

packages/desktop-client/src/hooks/useAccounts.ts Outdated Show resolved Hide resolved
dispatch(getAccounts());
}

return useSelector(state => state.queries.accounts);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💬 suggestion: ‏would be nice to also return the loading state. I bet it will be useful in some cases.

Suggested change
return useSelector(state => state.queries.accounts);
return { isLoading: !accountLoaded, data: useSelector(state => state.queries.accounts) };

packages/desktop-client/src/hooks/useCategories.ts Outdated Show resolved Hide resolved
packages/desktop-client/src/hooks/useDateFormat.ts Outdated Show resolved Hide resolved

export function usePayee(id: string) {
const payees = usePayees();
return useMemo(() => payees.find(p => p.id === id), [id]);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔨 warning:

Suggested change
return useMemo(() => payees.find(p => p.id === id), [id]);
return useMemo(() => payees.find(p => p.id === id), [id, payees]);

dispatch(getPayees());
}

return useSelector(state => state.queries.payees);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🥜 nitpick: ‏would be nice to return the loading state here too (but up to you to decide)

);
}

export function CachedAccounts({ children, idKey }) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👏 praise: ‏amazing!

Comment on lines 117 to 124
useEffect(() => {
actions.getAccounts();
actions.getCategories();
actions.getPayees();
}, []);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❓ question: ‏do we need these if the data is loaded automatically where the hook is used?

Comment on lines -60 to +64
send('backups-get', { id: budgetId }).then(setBackups);
}, [budgetId]);
send('backups-get', { id: budgetIdToLoad }).then(setBackups);
}, [budgetIdToLoad]);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❓ question: ‏why do we need this functional change?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Originally, the budget id was being retrieved by the root Modals component and passed to this component. I just moved that retrieval logic to this component.

export function useLocalPref<K extends keyof LocalPrefs>(
prefName: K,
): LocalPrefs[K] {
return useSelector(state => state.prefs.local?.[prefName]);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🥜 nitpick: ‏my wishlist item (but don't feel pressured to actually implement this) - for these hooks to have both setter & getter and a default value.

const [myPref, setMyPref] = useLocalPref('budget.collapsed', true);

// Usage:
console.log('my pref:', myPref);
<Button onClick={(someNewValue) => setMyPref(someNewValue)}>Click me</Button>

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a good idea! Implemented this :)

@trafico-bot trafico-bot bot added ⚠️ Changes requested Pull Request needs changes before it can be reviewed again and removed 🔍 Ready for Review labels Feb 3, 2024
@trafico-bot trafico-bot bot added 🔍 Ready for Review and removed ⚠️ Changes requested Pull Request needs changes before it can be reviewed again labels Feb 6, 2024
@joel-jeremy joel-jeremy force-pushed the more-hooks branch 2 times, most recently from fd9efe5 to 7968fac Compare February 8, 2024 06:46
@joel-jeremy
Copy link
Contributor Author

What changed in the VRT screenshot is the default visible column on smaller screens. This defaults to the Budgeted column instead of the Spent. I've also removed some props that were passed in several layers deep in favor of the new hooks.

MatissJanis
MatissJanis previously approved these changes Feb 11, 2024
Copy link
Member

@MatissJanis MatissJanis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Much cleaner! Great job!

packages/desktop-client/src/style/theme.tsx Outdated Show resolved Hide resolved
Co-authored-by: Matiss Janis Aboltins <matiss@mja.lv>
@joel-jeremy joel-jeremy merged commit 08cbdab into master Feb 12, 2024
19 checks passed
@joel-jeremy joel-jeremy deleted the more-hooks branch February 12, 2024 15:07
@trafico-bot trafico-bot bot added ✨ Merged Pull Request has been merged successfully and removed ✅ Approved labels Feb 12, 2024
FlorianLang06 pushed a commit to FlorianLang06/actual that referenced this pull request Mar 7, 2024
* Hooks for frequently made operations

* Release notes

* Fix typecheck errors

* Remove useGlobalPrefs

* Add null checks

* Fix showCleared pref

* Add loaded flag for categories, accounts and payees state

* Refactor to reduce unnecessary states

* Fix eslint errors

* Fix hooks deps

* Add useEffect

* Fix typecheck error

* Set local and global pref hooks

* Fix lint error

* VRT

* Fix typecheck error

* Remove eager loading

* Fix typecheck error

* Fix typo

* Fix typecheck error

* Update useTheme

* Typecheck errors

* Typecheck error

* defaultValue

* Explicitly check undefined

* Remove useGlobalPref and useLocalPref defaults

* Fix default prefs

* Default value

* Fix lint error

* Set default theme

* Default date format in Account

* Update packages/desktop-client/src/style/theme.tsx

Co-authored-by: Matiss Janis Aboltins <matiss@mja.lv>

---------

Co-authored-by: Matiss Janis Aboltins <matiss@mja.lv>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
✨ Merged Pull Request has been merged successfully
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants