Projects/App.js: memoize all the things

We do not need or want to recalculate all of this stuff on every render of App.js. In particular, this caused the entire Tabs bar and TabAction (with associated MiniFilterBar) to be removed from the DOM and re-added on every render. This caused problems with the openerRef being passed into TextFilter. Upon closer inspection, I was also able to remove all props from TextFilter. None of these needed to be passed in. The openerRef was being set and used entirely within the component, and managing the state from outside the component caused problems rather than solving them.
AutarkLabs · Nov 27, 2019 · ece86b8 · ece86b8
1 parent 5d8d8ce
commit ece86b8
Show file tree

Hide file tree

Showing 3 changed files with 81 additions and 77 deletions.
diff --git a/apps/projects/app/App.js b/apps/projects/app/App.js
@@ -1,4 +1,4 @@
-import React, { useEffect, useRef, useState } from 'react'
+import React, { useCallback, useEffect, useMemo, useState } from 'react'
 import { ApolloProvider } from 'react-apollo'
 import styled from 'styled-components'
 import { useAragonApi } from './api-react'
@@ -35,6 +35,8 @@ import { DecoratedReposProvider } from './context/DecoratedRepos'
 import { IssueFiltersProvider } from './context/IssueFilters'
 import { TextFilter } from './components/Shared/FilterBar/TextFilter'
 
+const noop = () => {}
+
 const App = () => {
   const { api, appState } = useAragonApi()
   const [ activeIndex, setActiveIndex ] = useState(0)
@@ -43,8 +45,7 @@ const App = () => {
   const [ panel, setPanel ] = useState(null)
   const [ panelProps, setPanelProps ] = useState(null)
   const [ popupRef, setPopupRef ] = useState(null)
-  const [ textFilterVisible, setTextFilterVisible ] = useState(false)
-  const textFilterOpenerApp = useRef(null)
+  const { setupNewIssue, setupNewProject, filters: filtersPanel } = usePanelManagement()
 
   const {
     repos = [],
@@ -67,7 +68,7 @@ const App = () => {
     window.close()
   })
 
-  const handlePopupMessage = async message => {
+  const handlePopupMessage = useCallback(async message => {
     if (message.data.from !== 'popup') return
     if (message.data.name === 'code') {
       // TODO: Optimize the listeners lifecycle, ie: remove on unmount
@@ -90,98 +91,107 @@ const App = () => {
         })
       }
     }
-  }
+  }, [api])
 
-  const changeActiveIndex = index => setActiveIndex(index)
+  const changeActiveIndex = useCallback(index => setActiveIndex(index), [])
 
-  const closePanel = () => {
+  const closePanel = useCallback(() => {
     setPanel(null)
     setPanelProps(null)
-  }
+  }, [])
 
-  const configurePanel = {
+  const configurePanel = useMemo(() => ({
     setActivePanel: p => setPanel(p),
     setPanelProps: p => setPanelProps(p),
-  }
+  }), [])
 
-  const handleGithubSignIn = () => {
+  const handleGithubSignIn = useCallback(() => {
     // The popup is launched, its ref is checked and saved in the state in one step
     setGithubLoading(true)
 
     setPopupRef(githubPopup(popupRef))
 
     // Listen for the github redirection with the auth-code encoded as url param
     window.addEventListener('message', handlePopupMessage)
-  }
+  }, [handlePopupMessage])
 
-  const handleResolveLocalIdentity = address => {
+  const handleResolveLocalIdentity = useCallback(address => {
     return api.resolveAddressIdentity(address).toPromise()
-  }
+  }, [api])
 
-  const handleShowLocalIdentityModal = address => {
+  const handleShowLocalIdentityModal = useCallback(address => {
     return api
       .requestAddressIdentityModification(address)
       .toPromise()
-  }
+  }, [api])
+
+  const tabs = useMemo(() => {
+    const newTabs = [
+      {
+        name: 'Overview',
+        body: Overview,
+        action: <Button mode="strong" icon={<IconPlus />} onClick={setupNewProject} label="New project" />,
+      },
+      {
+        name: 'Settings',
+        body: Settings,
+        action: null,
+      },
+    ]
+
+
+    if (repos.length) {
+      newTabs.splice(1, 0, {
+        name: 'Issues',
+        body: Issues,
+        action: <>
+          {!selectedIssueId && (
+            <MiniFilterBar>
+              <Button icon={<IconFilter />} onClick={filtersPanel} label="Filters Panel" />
+              <TextFilter />
+            </MiniFilterBar>
+          )}
+          <Button mode="strong" icon={<IconPlus />} onClick={setupNewIssue} label="New issue" />
+        </>
+      })
+    }
+
+    return newTabs
+  }, [ repos.length, selectedIssueId ])
+
+  const TabComponent = useMemo(
+    () => tabs[activeIndex].body,
+    [ tabs, activeIndex ]
+  )
+  const TabAction = useMemo(
+    () => tabs[activeIndex].action,
+    [ tabs, activeIndex ]
+  )
 
-  const noop = () => {}
   if (githubLoading) {
     return (
       <EmptyWrapper>
         <LoadingAnimation />
       </EmptyWrapper>
     )
-  } else if (github.status === STATUS.INITIAL) {
+  }
+
+  if (github.status === STATUS.INITIAL) {
     return (
       <Main>
         <Unauthorized onLogin={handleGithubSignIn} isSyncing={isSyncing} />
       </Main>
     )
-  } else if (github.status === STATUS.FAILED) {
+  }
+
+  if (github.status === STATUS.FAILED) {
     return (
       <Main>
         <Error action={noop} />
       </Main>
     )
   }
 
-  // Tabs are not fixed
-  const tabs = [{ name: 'Overview', body: Overview }]
-  if (repos.length)
-    tabs.push({ name: 'Issues', body: Issues })
-  tabs.push({ name: 'Settings', body: Settings })
-
-  const activateTextFilter = () => setTextFilterVisible(true)
-
-  // Determine current tab details
-  const TabComponent = tabs[activeIndex].body
-  const TabAction = () => {
-    const { setupNewIssue, setupNewProject, filters: filtersPanel } = usePanelManagement()
-
-    switch (tabs[activeIndex].name) {
-    case 'Overview': return (
-      <Button mode="strong" icon={<IconPlus />} onClick={setupNewProject} label="New project" />
-    )
-    case 'Issues': return (
-      <>
-        {!selectedIssueId && (
-          <MiniFilterBar>
-            <Button icon={<IconFilter />} onClick={filtersPanel} label="Filters Panel" />
-            <TextFilter
-              onClick={activateTextFilter}
-              visible={textFilterVisible}
-              openerRef={textFilterOpenerApp}
-              setVisible={setTextFilterVisible}
-            />
-          </MiniFilterBar>
-        )}
-        <Button mode="strong" icon={<IconPlus />} onClick={setupNewIssue} label="New issue" />
-      </>
-    )
-    default: return null
-    }
-  }
-
   return (
     <Main>
       <ApolloProvider client={client}>
@@ -194,9 +204,7 @@ const App = () => {
               <IssueFiltersProvider>
                 <Header
                   primary="Projects"
-                  secondary={
-                    <TabAction />
-                  }
+                  secondary={TabAction}
                 />
                 <ErrorBoundary>
                   {selectedIssueId

diff --git a/apps/projects/app/components/Shared/FilterBar/FilterBar.js b/apps/projects/app/components/Shared/FilterBar/FilterBar.js
@@ -184,11 +184,9 @@ const FilterBar = ({
 }) => {
   const [ sortMenuVisible, setSortMenuVisible ] = useState(false)
   const [ actionsMenuVisible, setActionsMenuVisible ] = useState(false)
-  const [ textFilterVisible, setTextFilterVisible ] = useState(false)
   const [ filtersDisplayNumber, setFiltersDisplayNumber ] = useState(10)
   const actionsOpener = useRef(null)
   const sortersOpener = useRef(null)
-  const textFilterOpener = useRef(null)
   const mainFBRef = useRef(null)
   const rightFBRef = useRef(null)
   const { availableFilters, activeFiltersCount } = useIssueFilters()
@@ -212,7 +210,6 @@ const FilterBar = ({
   const actionsClickHandler = () =>
     selectedIssues.length && setActionsMenuVisible(true)
 
-  const activateTextFilter = () => setTextFilterVisible(true)
   const activateSort = () => setSortMenuVisible(true)
 
   return (
@@ -240,12 +237,7 @@ const FilterBar = ({
         </FilterBarMainLeft>
 
         <FilterBarMainRight ref={rightFBRef}>
-          <TextFilter
-            onClick={activateTextFilter}
-            visible={textFilterVisible}
-            openerRef={textFilterOpener}
-            setVisible={setTextFilterVisible}
-          />
+          <TextFilter />
 
           <Button icon={<IconSort />} display="icon" onClick={activateSort} ref={sortersOpener} label="Sort by" />
           <SortPopover visible={sortMenuVisible} opener={sortersOpener.current} setVisible={setSortMenuVisible}
@@ -312,6 +304,7 @@ const FilterBarCard = styled(Card)`
   height: auto;
   padding: 12px;
   margin-bottom: 16px;
+
   @media only screen and (max-width: 514px) {
     display: none;
   }

diff --git a/apps/projects/app/components/Shared/FilterBar/TextFilter.js b/apps/projects/app/components/Shared/FilterBar/TextFilter.js
@@ -1,4 +1,4 @@
-import React from 'react'
+import React, { useRef, useState } from 'react'
 import PropTypes from 'prop-types'
 import {
   Button,
@@ -51,15 +51,24 @@ TextFilterPopover.propTypes = {
   setVisible: PropTypes.func.isRequired,
 }
 
-export const TextFilter = ({ visible, setVisible, openerRef, onClick }) => {
+export const TextFilter = () => {
   const { layoutName } = useLayout()
+  const [ visible, setVisible ] = useState(false)
+  const openerRef = useRef()
 
   if (layoutName === 'large') return (
     <TextFilterInput />
   )
+
   return (
     <>
-      <Button icon={<IconSearch />} display="icon" onClick={onClick} ref={openerRef} label="Text Filter" />
+      <Button
+        display="icon"
+        icon={<IconSearch />}
+        label="Text Filter"
+        onClick={() => setVisible(true)}
+        ref={openerRef}
+      />
       <TextFilterPopover
         visible={visible}
         opener={openerRef.current}
@@ -68,9 +77,3 @@ export const TextFilter = ({ visible, setVisible, openerRef, onClick }) => {
     </>
   )
 }
-TextFilter.propTypes = {
-  visible: PropTypes.bool.isRequired,
-  openerRef: PropTypes.object,
-  setVisible: PropTypes.func.isRequired,
-  onClick: PropTypes.func.isRequired,
-}