Merge pull request #3683 from mrfelton/perf/activity-loading

Improvements for activity list

Author: mrfelton

renderer/components/Activity/ActivityActions/ActivityActions.js

@@ -16,6 +16,7 @@ const ActivityActions = ({
   changeFilter,
   reloadActivityHistory,
   updateSearchText,
+  isPageLoading,
   isCustomFilter,
   intl,
   ...rest
@@ -39,7 +40,7 @@ const ActivityActions = ({
           mx={3}
         />
 
-        <ActivityRefresh mx={3} onClick={reloadActivityHistory} />
+        <ActivityRefresh isPageLoading={isPageLoading} mx={3} onClick={reloadActivityHistory} />
       </Flex>
     </Flex>
   </Card>
@@ -51,6 +52,7 @@ ActivityActions.propTypes = {
   filters: PropTypes.array.isRequired,
   intl: intlShape.isRequired,
   isCustomFilter: PropTypes.bool,
+  isPageLoading: PropTypes.bool,
   reloadActivityHistory: PropTypes.func.isRequired,
   searchText: PropTypes.string,
   updateSearchText: PropTypes.func.isRequired,

renderer/components/Activity/ActivityActions/ActivityRefresh.js

@@ -1,21 +1,26 @@
 import React from 'react'
 import PropTypes from 'prop-types'
-import { injectIntl } from 'react-intl'
+import { useIntl } from 'react-intl'
 import Refresh from 'components/Icon/Refresh'
 import { ActionButton } from 'components/UI'
 import messages from './messages'
 
-const ActivityRefresh = injectIntl(({ intl, onClick, ...rest }) => (
-  <ActionButton
-    hint={intl.formatMessage({ ...messages.refresh_button_hint })}
-    onClick={onClick}
-    {...rest}
-  >
-    <Refresh height="16px" width="16px" />
-  </ActionButton>
-))
+const ActivityRefresh = ({ isPageLoading, onClick, ...rest }) => {
+  const { formatMessage } = useIntl()
+  return (
+    <ActionButton
+      hint={formatMessage({ ...messages.refresh_button_hint })}
+      isLoading={isPageLoading}
+      onClick={onClick}
+      {...rest}
+    >
+      <Refresh height="16px" width="16px" />
+    </ActionButton>
+  )
+}
 
 ActivityRefresh.propTypes = {
+  isPageLoading: PropTypes.bool,
   onClick: PropTypes.func.isRequired,
 }
 

renderer/components/App/App.js

@@ -54,26 +54,15 @@ const App = ({
   willShowLnurlWithdrawPrompt,
 }) => {
   /**
-   * App scheduler / polling service setup. Add new app-wide polls here
+   * General app initialization.
    */
   useEffect(() => {
-    /**
-     * Fetch node data on an exponentially incrementing backoff schedule so that when the app is first mounted, we fetch
-     * node data quite frequently but as time goes on the frequency is reduced to a max of PEERS_MAX_REFETCH_INTERVAL
-     */
-    appScheduler.addTask({
-      task: () => !isSyncedToGraph && fetchDescribeNetwork(),
-      taskId: 'fetchNetworkData',
-      baseDelay: PEERS_INITIAL_REFETCH_INTERVAL,
-      maxDelay: PEERS_MAX_REFETCH_INTERVAL,
-      backoff: PEERS_REFETCH_BACKOFF_SCHEDULE,
-    })
-    appScheduler.addTask({
-      task: updateAutopilotNodeScores,
-      taskId: 'updateAutopilotNodeScores',
-      baseDelay: AUTOPILOT_SCORES_REFRESH_INTERVAL,
-    })
     if (activeWalletSettings.type === 'local') {
+      appScheduler.addTask({
+        task: updateAutopilotNodeScores,
+        taskId: 'updateAutopilotNodeScores',
+        baseDelay: AUTOPILOT_SCORES_REFRESH_INTERVAL,
+      })
       appScheduler.addTask({
         task: () => fetchTransactions(true),
         taskId: 'fetchTransactions',
@@ -101,7 +90,6 @@ const App = ({
   }, [
     activeWalletSettings,
     initActivityHistory,
-    isSyncedToGraph,
     fetchDescribeNetwork,
     fetchPeers,
     fetchSuggestedNodes,
@@ -112,6 +100,27 @@ const App = ({
     updateAutopilotNodeScores,
   ])
 
+  /**
+   * Fetch node data on an exponentially incrementing backoff schedule so that when the app is first mounted, we fetch
+   * node data quite frequently but as time goes on the frequency is reduced to a max of PEERS_MAX_REFETCH_INTERVAL
+   */
+  useEffect(() => {
+    if (isSyncedToGraph) {
+      appScheduler.removeTask('fetchNetworkData')
+    } else {
+      appScheduler.addTask({
+        task: () => fetchDescribeNetwork(),
+        taskId: 'fetchNetworkData',
+        baseDelay: PEERS_INITIAL_REFETCH_INTERVAL,
+        maxDelay: PEERS_MAX_REFETCH_INTERVAL,
+        backoff: PEERS_REFETCH_BACKOFF_SCHEDULE,
+      })
+    }
+  }, [isSyncedToGraph, fetchDescribeNetwork])
+
+  /**
+   * Lnurl handlers.
+   */
   useEffect(() => {
     if (lnurlAuthParams && !willShowLnurlAuthPrompt) {
       finishLnurlAuth()

renderer/components/Home/Home.js

@@ -35,6 +35,7 @@ class Home extends React.Component {
     isWalletUnlockerGrpcActive: PropTypes.bool.isRequired,
     lndConnect: PropTypes.string,
     putWallet: PropTypes.func.isRequired,
+    resetApp: PropTypes.func.isRequired,
     setActiveWallet: PropTypes.func.isRequired,
     setIsWalletOpen: PropTypes.func.isRequired,
     setUnlockWalletError: PropTypes.func.isRequired,
@@ -77,6 +78,7 @@ class Home extends React.Component {
       wallets,
       setActiveWallet,
       clearStartLndError,
+      resetApp,
       showError,
       stopLnd,
       isNeutrinoRunning,
@@ -135,6 +137,7 @@ class Home extends React.Component {
                     isStartingLnd={isStartingLnd}
                     isWalletUnlockerGrpcActive={isWalletUnlockerGrpcActive}
                     putWallet={putWallet}
+                    resetApp={resetApp}
                     showError={showError}
                     showNotification={showNotification}
                     startLnd={startLnd}

renderer/components/Home/WalletLauncher.js

@@ -201,6 +201,7 @@ class WalletLauncher extends React.Component {
     isStartingLnd: PropTypes.bool.isRequired,
     isWalletUnlockerGrpcActive: PropTypes.bool.isRequired,
     putWallet: PropTypes.func.isRequired,
+    resetApp: PropTypes.func.isRequired,
     showError: PropTypes.func.isRequired,
     showNotification: PropTypes.func.isRequired,
     startLnd: PropTypes.func.isRequired,
@@ -259,8 +260,9 @@ class WalletLauncher extends React.Component {
 
   launchWallet = async () => {
     try {
-      const { startLnd, wallet } = this.props
+      const { resetApp, startLnd, wallet } = this.props
       // discard settings edits and just launch current saved config
+      resetApp()
       return await startLnd(wallet)
     } catch (e) {
       // this error is handled via startLndErrors mechanism

renderer/components/UI/ActionButton.js

@@ -16,7 +16,7 @@ StyledButton.propTypes = {
   active: PropTypes.bool,
 }
 
-const ActionButton = ({ children, hint, onClick, timeout, ...rest }) => {
+const ActionButton = ({ children, hint, isLoading, onClick, timeout = 1000, ...rest }) => {
   const [status, setStatus] = useState(null)
   const buttonRef = useRef()
 
@@ -49,20 +49,17 @@ const ActionButton = ({ children, hint, onClick, timeout, ...rest }) => {
       variant="secondary"
       {...rest}
     >
-      {status === 'fetching' ? <Spinner height="16px" width="16px" /> : children}
+      {status === 'fetching' || isLoading ? <Spinner height="16px" width="16px" /> : children}
     </StyledButton>
   )
 }
 
 ActionButton.propTypes = {
   children: PropTypes.node,
   hint: PropTypes.node,
+  isLoading: PropTypes.bool,
   onClick: PropTypes.func.isRequired,
   timeout: PropTypes.number,
 }
 
-ActionButton.defaultProps = {
-  timeout: 1000,
-}
-
 export default ActionButton

renderer/containers/Activity/ActivityActions.js

@@ -18,6 +18,7 @@ const mapStateToProps = state => ({
   filters: activitySelectors.filters(state),
   searchText: activitySelectors.searchText(state),
   isCustomFilter: activitySelectors.isCustomFilter(state),
+  isPageLoading: activitySelectors.isPageLoading(state),
 })
 
 export default connect(mapStateToProps, mapDispatchToProps)(ActivityActions)

renderer/containers/Home/Home.js

@@ -1,5 +1,6 @@
 import React from 'react'
 import { connect } from 'react-redux'
+import { resetApp } from 'reducers/app'
 import { neutrinoSelectors } from 'reducers/neutrino'
 import {
   setActiveWallet,
@@ -51,6 +52,7 @@ const mapDispatchToProps = {
   clearStartLndError,
   stopLnd,
   putWallet,
+  resetApp,
   showNotification,
   startLnd,
   unlockWallet,

renderer/reducers/activity/reducer.js

@@ -1,5 +1,4 @@
 import { send } from 'redux-electron-ipc'
-import groupBy from 'lodash/groupBy'
 import createReducer from '@zap/utils/createReducer'
 import { getIntl } from '@zap/i18n'
 import { mainLog } from '@zap/utils/log'
@@ -11,8 +10,9 @@ import { settingsSelectors } from 'reducers/settings'
 import { fetchBalance } from 'reducers/balance'
 import { fetchChannels } from 'reducers/channels'
 import { fetchInfo, infoSelectors } from 'reducers/info'
+import { walletSelectors } from 'reducers/wallet'
 import { showError, showNotification } from 'reducers/notification'
-import { createActivityPaginator, getItemType } from './utils'
+import { createActivityPaginator } from './utils'
 import { hasNextPage, isPageLoading } from './selectors'
 import messages from './messages'
 import * as constants from './constants'
@@ -77,17 +77,21 @@ const initialState = {
 // activity paginator object. must be reset for each wallet login
 /** @type {Function | null} */
 let paginator = null
-let loadedPages = 0
 
 /**
  * getPaginator - Returns current activity paginator object. This acts as a singleton
  * and creates paginator if it's not initialized.
  *
+ * @param {{
+ *  invoiceHandler: function,
+ *  paymentHandler: function,
+ *  transactionHandler: function
+ * }} handlers Pagination handlers
  * @returns {Function} Paginator
  */
-export const getPaginator = () => {
+export const getPaginator = handlers => {
   if (!paginator) {
-    paginator = createActivityPaginator()
+    paginator = createActivityPaginator(handlers)
   }
   return paginator
 }
@@ -237,22 +241,12 @@ export const hideActivityModal = () => dispatch => {
  */
 export const resetActivity = () => () => {
   paginator = null
-  loadedPages = 0
-}
-
-/**
- * resetPaginator - Reset activity paginator.
- *
- * @returns {() => void} Thunk
- */
-export const resetPaginator = () => () => {
-  paginator = null
 }
 
 /**
  * loadPage - Loads next activity page if it's available.
  *
- * @param {boolean} reload Boolean indicating if this page load is part of a reload.
+ * @param {boolean} reload Boolean indicating wether to load first page.
  * @returns {(dispatch:Function, getState:Function) => Promise<void>} Thunk
  */
 export const loadPage = (reload = false) => async (dispatch, getState) => {
@@ -262,38 +256,45 @@ export const loadPage = (reload = false) => async (dispatch, getState) => {
   dispatch(setPageLoading(true))
 
   await dispatch(fetchInfo())
-  const config = settingsSelectors.currentConfig(getState())
-  const blockHeight = infoSelectors.blockHeight(getState())
-  const thisPaginator = getPaginator()
+  const state = getState()
+  const config = settingsSelectors.currentConfig(state)
+  const blockHeight = infoSelectors.blockHeight(state)
+  const activeWallet = walletSelectors.activeWallet(state)
+
+  // Checks to see if the currently active wallet is the same one as when the page load started.
+  const isStllActiveWallet = () => walletSelectors.activeWallet(getState()) === activeWallet
+
+  // It's possible that the grpc response is received after the wallet has been disconnected.
+  // See https://github.com/grpc/grpc-node/issues/1603
+  // Ensure that the items received belong to the currently active wallet.
+  const shouldUpdate = items => items && isStllActiveWallet()
+
+  const handlers = {
+    invoiceHandler: items => shouldUpdate(items) && dispatch(receiveInvoices(items)),
+    paymentHandler: items => shouldUpdate(items) && dispatch(receivePayments(items)),
+    transactionHandler: items => shouldUpdate(items) && dispatch(receiveTransactions(items)),
+  }
+  const thisPaginator = reload ? createActivityPaginator(handlers) : getPaginator(handlers)
 
-  if (reload || hasNextPage(getState())) {
+  if (hasNextPage(getState())) {
     const { pageSize } = config.activity
-    const { items, hasNextPage: paginatorHasNextPage } = await thisPaginator(pageSize, blockHeight)
-
-    if (!reload) {
-      loadedPages += 1
-      dispatch({ type: SET_HAS_NEXT_PAGE, value: paginatorHasNextPage })
+    const { hasNextPage } = await thisPaginator(pageSize, blockHeight)
+    if (!isStllActiveWallet()) {
+      return
     }
-
-    const { invoices, payments, transactions } = groupBy(items, getItemType)
-
-    invoices && dispatch(receiveInvoices(invoices))
-    payments && dispatch(receivePayments(payments))
-    transactions && dispatch(receiveTransactions(transactions))
+    dispatch({ type: SET_HAS_NEXT_PAGE, value: hasNextPage })
   }
 
   dispatch(setPageLoading(false))
 }
 
 /**
- * reloadPages - Reloads all already loaded activity pages.
+ * reloadActivityHead - Reloads activity head.
  *
  * @returns {(dispatch:Function) => Promise<void>} Thunk
  */
-export const reloadPages = () => async dispatch => {
-  const pageCount = loadedPages
-  mainLog.debug(`reloading ${pageCount} activity pages`)
-  dispatch(resetPaginator())
+export const reloadActivityHead = () => async dispatch => {
+  mainLog.debug(`reloading activity pages`)
   await dispatch(loadPage(true))
 }
 
@@ -321,7 +322,7 @@ export const reloadActivityHistory = () => async dispatch => {
   dispatch({ type: FETCH_ACTIVITY_HISTORY })
   try {
     await Promise.all([
-      dispatch(reloadPages()),
+      dispatch(reloadActivityHead()),
       dispatch(fetchChannels()),
       dispatch(fetchBalance()),
     ])

renderer/reducers/activity/selectors.js

@@ -278,11 +278,12 @@ export const isCustomFilter = createSelector(filter, currentFilter => {
  * All selectors to export.
  */
 export default {
+  errorDialogDetails,
   filter,
   filters,
   searchText,
   hasNextPage,
-  errorDialogDetails,
+  isPageLoading,
   isErrorDialogOpen,
   currentActivity,
   activityModalItem,