From bd4255108d65f32508a9822ea0c0b1c75ec49f64 Mon Sep 17 00:00:00 2001
From: Alejandro Celaya <alejandro@alejandrocelaya.com>
Date: Sat, 4 Apr 2020 12:58:04 +0200
Subject: [PATCH] Improved VisitsTable performance by memoizing visits lists

---
 src/utils/Message.js         |  4 ++--
 src/utils/helpers/numbers.js |  4 ++++
 src/visits/ShortUrlVisits.js |  5 +++--
 src/visits/VisitsTable.js    | 36 ++++++++++++++----------------------
 4 files changed, 23 insertions(+), 26 deletions(-)

diff --git a/src/utils/Message.js b/src/utils/Message.js
index 67b8c087..2aa723f8 100644
--- a/src/utils/Message.js
+++ b/src/utils/Message.js
@@ -35,8 +35,8 @@ const Message = ({ children, loading = false, noMargin = false, type = 'default'
       <Card className={cardClasses} body>
         <h3 className={classNames('text-center mb-0', getTextClassForType(type))}>
           {loading && <FontAwesomeIcon icon={preloader} spin />}
-          {loading && !children && <span className="ml-2">Loading...</span>}
-          {children}
+          {loading && <span className="ml-2">{children || 'Loading...'}</span>}
+          {!loading && children}
         </h3>
       </Card>
     </div>
diff --git a/src/utils/helpers/numbers.js b/src/utils/helpers/numbers.js
index 7cfe751b..b9e0757f 100644
--- a/src/utils/helpers/numbers.js
+++ b/src/utils/helpers/numbers.js
@@ -1,4 +1,8 @@
 const TEN_ROUNDING_NUMBER = 10;
 const { ceil } = Math;
 
+const formatter = new Intl.NumberFormat('en-US');
+
+export const prettify = (number) => formatter.format(number);
+
 export const roundTen = (number) => ceil(number / TEN_ROUNDING_NUMBER) * TEN_ROUNDING_NUMBER;
diff --git a/src/visits/ShortUrlVisits.js b/src/visits/ShortUrlVisits.js
index d18dbd66..5a5ed3ec 100644
--- a/src/visits/ShortUrlVisits.js
+++ b/src/visits/ShortUrlVisits.js
@@ -78,6 +78,7 @@ const ShortUrlVisits = (
   render() {
     const { shortUrlVisits, shortUrlDetail } = this.props;
     const { visits, loading, loadingLarge, error } = shortUrlVisits;
+    const showTableControls = !loading && visits.length > 0;
 
     const renderVisitsContent = () => {
       if (loading) {
@@ -166,7 +167,7 @@ const ShortUrlVisits = (
               />
             </div>
             <div className="col-lg-4 col-xl-6 mt-4 mt-lg-0">
-              {visits.length > 0 && (
+              {showTableControls && (
                 <Button
                   outline
                   block={this.state.isMobileDevice}
@@ -179,7 +180,7 @@ const ShortUrlVisits = (
           </div>
         </section>
 
-        {!loading && visits.length > 0 && (
+        {showTableControls && (
           <Collapse
             isOpen={this.state.showTable}
 
diff --git a/src/visits/VisitsTable.js b/src/visits/VisitsTable.js
index 44880cf8..6a3c5fb1 100644
--- a/src/visits/VisitsTable.js
+++ b/src/visits/VisitsTable.js
@@ -1,4 +1,4 @@
-import React, { useEffect, useState } from 'react';
+import React, { useEffect, useMemo, useState } from 'react';
 import PropTypes from 'prop-types';
 import Moment from 'react-moment';
 import classNames from 'classnames';
@@ -13,6 +13,7 @@ import SimplePaginator from '../common/SimplePaginator';
 import SearchField from '../utils/SearchField';
 import { browserFromUserAgent, extractDomain, osFromUserAgent } from '../utils/helpers/visits';
 import { determineOrderDir } from '../utils/utils';
+import { prettify } from '../utils/helpers/numbers';
 import { visitType } from './reducers/shortUrlVisits';
 import './VisitsTable.scss';
 
@@ -56,39 +57,30 @@ const normalizeVisits = map(({ userAgent, date, referer, visitLocation }) => ({
 }));
 
 const VisitsTable = ({ visits, onVisitSelected, isSticky = false, matchMedia = window.matchMedia }) => {
-  const allVisits = normalizeVisits(visits);
   const headerCellsClass = classNames('visits-table__header-cell', {
     'visits-table__sticky': isSticky,
   });
   const matchMobile = () => matchMedia('(max-width: 767px)').matches;
 
   const [ selectedVisit, setSelectedVisit ] = useState(undefined);
+  const [ isMobileDevice, setIsMobileDevice ] = useState(matchMobile());
   const [ page, setPage ] = useState(1);
   const [ searchTerm, setSearchTerm ] = useState(undefined);
   const [ order, setOrder ] = useState({ field: undefined, dir: undefined });
-  const [ currentPage, setCurrentPageVisits ] = useState(calculateVisits(allVisits, page, searchTerm, order));
-  const [ isMobileDevice, setIsMobileDevice ] = useState(matchMobile());
+  const allVisits = useMemo(() => normalizeVisits(visits), [ visits ]);
+  const currentPage = useMemo(() => calculateVisits(allVisits, page, searchTerm, order), [ page, searchTerm, order ]);
 
   const orderByColumn = (field) => () => setOrder({ field, dir: determineOrderDir(field, order.field, order.dir) });
-  const renderOrderIcon = (field) => {
-    if (!order.dir || order.field !== field) {
-      return null;
-    }
-
-    return (
-      <FontAwesomeIcon
-        icon={order.dir === 'ASC' ? caretUpIcon : caretDownIcon}
-        className="visits-table__header-icon"
-      />
-    );
-  };
+  const renderOrderIcon = (field) => order.dir && order.field === field && (
+    <FontAwesomeIcon
+      icon={order.dir === 'ASC' ? caretUpIcon : caretDownIcon}
+      className="visits-table__header-icon"
+    />
+  );
 
   useEffect(() => {
     onVisitSelected && onVisitSelected(selectedVisit);
   }, [ selectedVisit ]);
-  useEffect(() => {
-    setCurrentPageVisits(calculateVisits(allVisits, page, searchTerm, order));
-  }, [ page, searchTerm, order ]);
   useEffect(() => {
     const listener = () => setIsMobileDevice(matchMobile());
 
@@ -188,9 +180,9 @@ const VisitsTable = ({ visits, onVisitSelected, isSticky = false, matchMedia = w
                   })}
                 >
                   <div>
-                    Visits <b>{currentPage.start + 1}</b> to{' '}
-                    <b>{min(currentPage.end, currentPage.total)}</b> of{' '}
-                    <b>{currentPage.total}</b>
+                    Visits <b>{prettify(currentPage.start + 1)}</b> to{' '}
+                    <b>{prettify(min(currentPage.end, currentPage.total))}</b> of{' '}
+                    <b>{prettify(currentPage.total)}</b>
                   </div>
                 </div>
               </div>